🥟 🤜 🎆 Código do jogo Command & Conquer: bugs dos anos 90. Volume dois 👩🏾‍✈️ 👨🏿‍⚕️ ↙️

A americana Electronic Arts Inc (EA) divulgou o código-fonte dos jogos Command & Conquer: Tiberian Dawn e Command & Conquer: Red Alert. Várias dezenas de bugs foram encontrados no código-fonte usando o analisador PVS-Studio, portanto, dê boas-vindas à continuação da descrição dos defeitos encontrados.

Introdução

Command & Conquer é uma série de jogos de computador do gênero estratégia em tempo real. O primeiro jogo da série foi lançado em 1995. O código-fonte dos jogos foi publicado com o lançamento da coleção Command & Conquer Remastered .

Para localizar erros no código, foi utilizado o analisador PVS-Studio . É uma ferramenta para identificar erros e vulnerabilidades potenciais no código-fonte de programas escritos em C, C ++, C # e Java.

Link para o primeiro resumo de bug: " Command & Conquer Game : Bugs from the 90s. Volume One ".

Erros nas condições

V583 O operador '?:', Independentemente de sua expressão condicional, sempre retorna um e o mesmo valor: 3072. STARTUP.CPP 1136

void Read_Setup_Options( RawFileClass *config_file )
{
  ....
  ScreenHeight = ini.Get_Bool("Options", "Resolution", false) ? 3072 : 3072;
  ....
}

Acontece que os usuários não podem influenciar algumas configurações. Mais precisamente, fizeram algo, mas devido ao fato de o operador ternário sempre retornar um valor, na verdade nada mudou.

V590 Considere inspecionar a expressão 'i <8 && i <4'. A expressão é excessiva ou contém um erro de impressão. DLLInterface.cpp 2238

// Maximum number of multi players possible.
#define MAX_PLAYERS 8 // max # of players we can have

for (int i = 0; i < MAX_PLAYERS && i < 4; i++) {
  if (GlyphxPlayerIDs[i] == player_id) {
    MultiplayerStartPositions[i] = XY_Cell(x, y);
  }
}

Devido ao ciclo errado, a posição não é definida para todos os jogadores. Por um lado, vemos a constante MAX_PLAYERS 8 e assumimos que este é o número máximo de jogadores. Por outro lado, vemos a condição i <4 e o operador && . Portanto, o loop nunca faz 8 iterações. Provavelmente, no estágio inicial de desenvolvimento, o programador não usou constantes e, quando começou, esqueceu de remover os números antigos do código.

V648 A prioridade da operação '&&' é maior do que a da operação '||' Operação. INFANTRY.CPP 1003

void InfantryClass::Assign_Target(TARGET target)
{
  ....
  if (building && building->Class->IsCaptureable &&
    (GameToPlay != GAME_NORMAL || *building != STRUCT_EYE && Scenario < 13)) {
    Assign_Destination(target);
  }
  ....
}

Você pode tornar o código não óbvio (e provavelmente errôneo) simplesmente não especificando a prioridade das operações para os operadores || . e && . Não está totalmente claro aqui se isso é um erro ou não. Mas dada a qualidade geral do código desses projetos, vamos supor que aqui e em alguns outros lugares haja erros com a prioridade das operações:

A prioridade V648 da operação '&&' é maior do que a da operação '||' Operação. TEAM.CPP 456
A prioridade V648 da operação '&&' é maior do que a da operação '||' Operação. DISPLAY.CPP 1160
V648 A prioridade da operação '&&' é maior do que a da operação '||' Operação. DISPLAY.CPP 1571
V648 A prioridade da operação '&&' é maior do que a da operação '||' Operação. HOUSE.CPP 2594
A prioridade V648 da operação '&&' é maior do que a da operação '||' Operação. INIT.CPP 2541

V617 Considere inspecionar a condição. O argumento '((1L << STRUCT_CHRONOSPHERE))' do '|' operação bit a bit contém um valor diferente de zero. HOUSE.CPP 5089

typedef enum StructType : char {
  STRUCT_NONE=-1,
  STRUCT_ADVANCED_TECH,
  STRUCT_IRON_CURTAIN,
  STRUCT_WEAP,
  STRUCT_CHRONOSPHERE, // 3
  ....
}

#define  STRUCTF_CHRONOSPHERE (1L << STRUCT_CHRONOSPHERE)

UrgencyType HouseClass::Check_Build_Power(void) const
{
  ....
  if (State == STATE_THREATENED || State == STATE_ATTACKED) {
    if (BScan | (STRUCTF_CHRONOSPHERE)) {  // <=
      urgency = URGENCY_HIGH;
    }
  }
  ....
}

Para verificar se certos bits em uma variável estão definidos, use o operador &, não |. Devido a um erro de digitação neste trecho de código, a condição é sempre verdadeira.

V768 A constante de enumeração 'WWKEY_RLS_BIT' é usada como uma variável do tipo booleano. KEYBOARD.CPP 286

typedef enum {
  WWKEY_SHIFT_BIT = 0x100,
  WWKEY_CTRL_BIT  = 0x200,
  WWKEY_ALT_BIT   = 0x400,
  WWKEY_RLS_BIT   = 0x800,
  WWKEY_VK_BIT    = 0x1000,
  WWKEY_DBL_BIT   = 0x2000,
  WWKEY_BTN_BIT   = 0x8000,
} WWKey_Type;

int WWKeyboardClass::To_ASCII(int key)
{
  if ( key && WWKEY_RLS_BIT)
    return(KN_NONE);
  return(key);
}

Acho que, no parâmetro chave , eles queriam verificar um determinado bit especificado pela máscara WWKEY_RLS_BIT , mas cometeram um erro de digitação. O operador bit a bit &, em vez de &&, deve ter sido usado para verificar o código-chave.

Formatação suspeita

V523 A instrução 'then' é equivalente à instrução 'else'. RADAR.CPP 1827

void RadarClass::Player_Names(bool on)
{
  IsPlayerNames = on;
  IsToRedraw = true;
  if (on) {
    Flag_To_Redraw(true);
//    Flag_To_Redraw(false);
  } else {
    Flag_To_Redraw(true);   // force drawing of the plate
  }
}

Era uma vez, um desenvolvedor comentou sobre o código para depuração. Desde então, o código permaneceu um operador condicional com os mesmos operadores em diferentes ramos.

Exatamente os mesmos lugares foram encontrados mais dois:

V523 A instrução 'then' é equivalente à instrução 'else'. CELL.CPP 1792
V523 A instrução 'then' é equivalente à instrução 'else'. RADAR.CPP 2274

V705 É possível que o bloco 'else' tenha sido esquecido ou comentado, alterando a lógica de funcionamento do programa. NETDLG.CPP 1506

static int Net_Join_Dialog(void)
{
  ....
  /*...............................................................
  F4/SEND/'M' = edit a message
  ...............................................................*/
  if (Messages.Get_Edit_Buf()==NULL) {
    ....
  } else

  /*...............................................................
  If we're already editing a message and the user clicks on
  'Send', translate our input to a Return so Messages.Input() will
  work properly.
  ...............................................................*/
  if (input==(BUTTON_SEND | KN_BUTTON)) {
    input = KN_RETURN;
  }
  ....
}

Devido a um grande comentário, o desenvolvedor não viu o operador condicional indefinido acima. O resto da palavra-chave else forma uma construção else if com a condição abaixo , que é provavelmente uma alteração da lógica original.

V519 A variável 'ScoresPresent' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 539, 541. INIT.CPP 541

bool Init_Game(int , char *[])
{
  ....
  ScoresPresent = false;
//if (CCFileClass("SCORES.MIX").Is_Available()) {
    ScoresPresent = true;
    if (!ScoreMix) {
      ScoreMix = new MixFileClass("SCORES.MIX");
      ThemeClass::Scan();
    }
//}

Outro defeito potencial devido à refatoração inacabada. Agora não está claro se a variável ScoresPresent deve ser verdadeira ou ainda falsa .

Erros de desalocação de memória

V611 A memória foi alocada usando o operador 'new T []', mas foi liberada usando o operador 'delete'. Considere inspecionar este código. Provavelmente é melhor usar 'delete [] poke_data;'. CCDDE.CPP 410

BOOL Send_Data_To_DDE_Server (char *data, int length, int packet_type)
{
  ....
  char *poke_data = new char [length + 2*sizeof(int)]; // <=
  ....
  if(DDE_Class->Poke_Server( .... ) == FALSE) {
    CCDebugString("C&C95 - POKE failed!\n");
    DDE_Class->Close_Poke_Connection();
    delete poke_data;                                  // <=
    return (FALSE);
  }

  DDE_Class->Close_Poke_Connection();

  delete poke_data;                                    // <=

  return (TRUE);
}

O analisador detectou um erro relacionado ao fato de que a memória pode ser alocada e liberada de maneiras incompatíveis. Para liberar a memória alocada para a matriz, você deve ter usado o operador delete [] , não delete .

Havia vários desses lugares, e todos eles gradualmente prejudicaram o aplicativo em execução (jogo):

V611 A memória foi alocada usando o operador 'new T []', mas foi liberada usando o operador 'delete'. Considere inspecionar este código. Provavelmente é melhor usar 'delete [] poke_data;'. CCDDE.CPP 416
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] temp_buffer;'. INIT.CPP 1302
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] progresspalette;'. MAPSEL.CPP 795
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] grey2palette;'. MAPSEL.CPP 796
V611 A memória foi alocada usando o operador 'new T []', mas foi liberada usando o operador 'delete'. Considere inspecionar este código. Provavelmente é melhor usar 'delete [] poke_data;'. CCDDE.CPP 422
V611 A memória foi alocada usando o operador 'new T []', mas foi liberada usando o operador 'delete'. Considere inspecionar este código. Provavelmente é melhor usar 'delete [] temp_buffer;'. INIT.CPP 1139

V772 Chamar um operador 'delete' para um ponteiro void causará um comportamento indefinido. ENDING.CPP 254

void GDI_Ending(void)
{
  ....
  void * localpal = Load_Alloc_Data(CCFileClass("SATSEL.PAL"));
  ....
  delete [] localpal;
  ....
}

Os operadores delete e delete [] são separados por um motivo. Eles fazem um trabalho diferente de limpar a memória. E, ao usar um ponteiro sem tipo, o compilador não sabe para qual tipo de dados o ponteiro está apontando. No padrão de linguagem C ++, o comportamento do compilador é indefinido.

Uma série de avisos do analisador também foram encontrados deste tipo:

V772 Chamar um operador 'delete' para um ponteiro void causará um comportamento indefinido. HEAP.CPP 284
V772 Chamar um operador 'delete' para um ponteiro void causará um comportamento indefinido. INIT.CPP 728
V772 Chamar um operador 'delete' para um ponteiro void causará um comportamento indefinido. MIXFILE.CPP 134
V772 Chamar um operador 'delete' para um ponteiro void causará um comportamento indefinido. MIXFILE.CPP 391
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MSGBOX.CPP 423
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. SOUNDDLG.CPP 407
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. BUFFER.CPP 126
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. BUFF.CPP 162
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. BUFF.CPP 212
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. BFIOFILE.CPP 330
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. EVENT.CPP 934
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. HEAP.CPP 318
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. INIT.CPP 3851
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MIXFILE.CPP 130
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MIXFILE.CPP 430
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MIXFILE.CPP 447
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MIXFILE.CPP 481
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MSGBOX.CPP 461
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. QUEUE.CPP 2982
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. QUEUE.CPP 3167
V772 Chamar um operador 'delete' para um ponteiro void causará um comportamento indefinido. SOUNDDLG.CPP 406

V773 A função foi encerrada sem liberar o ponteiro 'progresspalette'. Um vazamento de memória é possível. MAPSEL.CPP 258

void Map_Selection(void)
{
  ....
  unsigned char *grey2palette    = new unsigned char[768];
  unsigned char *progresspalette = new unsigned char[768];
  ....
  scenario = Scenario + ((house == HOUSE_GOOD) ? 0 : 14);
  if (house == HOUSE_GOOD) {
    lastscenario = (Scenario == 14);
    if (Scenario == 15) return;
  } else {
    lastscenario = (Scenario == 12);
    if (Scenario == 13) return;
  }
  ....
}

"Se você não liberar memória alguma, com certeza não me enganarei ao escolher uma operadora!" - talvez o programador pensou.

Mas então ocorre um vazamento de memória, o que também é um erro. Em algum lugar no final da função, a memória é liberada, mas antes disso há muitos lugares onde ocorre a saída condicional da função, e a memória pelos ponteiros grey2palette e progresspalett não é liberada.

miscelânea

V570 A variável 'hdr-> MagicNumber' é atribuída a si mesma. COMBUF.CPP 806

struct CommHdr {
  unsigned short MagicNumber;
  unsigned char Code;
  unsigned long PacketID;
} *hdr;

void CommBufferClass::Mono_Debug_Print(int refresh)
{
  ....
  hdr = (CommHdr *)SendQueue[i].Buffer;
  hdr->MagicNumber = hdr->MagicNumber;
  hdr->Code = hdr->Code;
  ....
}

Dois campos da estrutura CommHdr são inicializados com seus próprios valores. Na minha opinião, uma operação inútil, mas muitas vezes realizada:

V570 A variável 'hdr-> Code' é atribuída a si mesma. COMBUF.CPP 807
V570 A variável 'hdr-> MagicNumber' é atribuída a si mesma. COMBUF.CPP 931
V570 A variável 'hdr-> Code' é atribuída a si mesma. COMBUF.CPP 932
V570 A variável 'hdr-> MagicNumber' é atribuída a si mesma. COMBUF.CPP 987
V570 A variável 'hdr-> Code' é atribuída a si mesma. COMBUF.CPP 988
V570 A variável 'obj' é atribuída a si mesma. MAP.CPP 1132
V570 A variável 'hdr-> MagicNumber' é atribuída a si mesma. COMBUF.CPP 910
V570 A variável 'hdr-> Code' é atribuída a si mesma. COMBUF.CPP 911
V570 A variável 'hdr-> MagicNumber' é atribuída a si mesma. COMBUF.CPP 1040
V570 A variável 'hdr-> Code' é atribuída a si mesma. COMBUF.CPP 1041
V570 A variável 'hdr-> MagicNumber' é atribuída a si mesma. COMBUF.CPP 1104
V570 A variável 'hdr-> Code' é atribuída a si mesma. COMBUF.CPP 1105
V570 A variável 'obj' é atribuída a si mesma. MAP.CPP 1279

A função V591 não vazia deve retornar um valor. HEAP.H 123

int FixedHeapClass::Free(void * pointer);

template<class T>
class TFixedHeapClass : public FixedHeapClass
{
  ....
  virtual int Free(T * pointer) {FixedHeapClass::Free(pointer);};
};

As funções da classe Free TFixedHeapClass nenhuma instrução de retorno do operador . O mais interessante é que a função chamada FixedHeapClass :: Free também tem um valor de retorno do tipo int . Provavelmente, o programador simplesmente se esqueceu de escrever a instrução de retorno e agora a função retorna um valor incompreensível.

V672 Provavelmente não há necessidade de criar a nova variável 'dano' aqui. Um dos argumentos da função possui o mesmo nome e este argumento é uma referência. Verifique as linhas: 1219, 1278. BUILDING.CPP 1278

ResultType BuildingClass::Take_Damage(int & damage, ....)
{
  ....
  if (tech && tech->IsActive && ....) {
    int damage = 500;
    tech->Take_Damage(damage, 0, WARHEAD_AP, source, forced);
  }
  ....
}

O parâmetro de dano é passado por referência. Portanto, mudanças nos valores desta variável são esperadas no corpo da função. Mas em um lugar, o desenvolvedor declarou uma variável com o mesmo nome. Por causa disso, o valor 500 será armazenado na variável local danos, ao invés de um parâmetro de função. Talvez um comportamento diferente fosse pretendido.

Outro lugar assim:

V672 Provavelmente não há necessidade de criar a nova variável 'dano' aqui. Um dos argumentos da função possui o mesmo nome e este argumento é uma referência. Verifique as linhas: 4031, 4068. TECHNO.CPP 4068

V762 É possível que uma função virtual tenha sido substituída incorretamente. Consulte o primeiro argumento da função 'Occupy_List' na classe derivada 'BulletClass' e na classe base 'ObjectClass'. BULLET.H 90

class ObjectClass : public AbstractClass
{
  ....
  virtual short const * Occupy_List(bool placement=false) const; // <=
  virtual short const * Overlap_List(void) const;
  ....
};

class BulletClass : public ObjectClass,
                    public FlyClass,
                    public FuseClass
{
  ....
  virtual short const * Occupy_List(void) const;                 // <=
  virtual short const * Overlap_List(void) const {return Occupy_List();};
  ....
};

O analisador detectou um erro potencial ao substituir a função virtual Occupy_List . Isso pode fazer com que as funções erradas sejam chamadas em tempo de execução.

Mais alguns lugares suspeitos:

V762 É possível que uma função virtual tenha sido substituída incorretamente. Consulte os qualificadores da função 'Ok_To_Move' na classe derivada 'TurretClass' e na classe base 'DriveClass'. TURRET.H 76
V762 É possível que uma função virtual tenha sido substituída incorretamente. Veja o quarto argumento da função 'Help_Text' na classe derivada 'HelpClass' e a classe base 'DisplayClass'. HELP.H 55
V762 É possível que uma função virtual tenha sido substituída incorretamente. Consulte o primeiro argumento da função 'Draw_It' na classe derivada 'MapEditClass' e na classe base 'HelpClass'. MAPEDIT.H 187
V762 It is possible a virtual function was overridden incorrectly. See first argument of function 'Occupy_List' in derived class 'AnimClass' and base class 'ObjectClass'. ANIM.H 80
V762 It is possible a virtual function was overridden incorrectly. See first argument of function 'Overlap_List' in derived class 'BulletClass' and base class 'ObjectClass'. BULLET.H 102
V762 It is possible a virtual function was overridden incorrectly. See qualifiers of function 'Remap_Table' in derived class 'BuildingClass' and base class 'TechnoClass'. BUILDING.H 281
V762 It is possible a virtual function was overridden incorrectly. See fourth argument of function 'Help_Text' in derived class 'HelpClass' and base class 'DisplayClass'. HELP.H 58
V762 É possível que uma função virtual tenha sido substituída incorretamente. Consulte o primeiro argumento da função 'Overlap_List' na classe derivada 'AnimClass' e na classe base 'ObjectClass'. ANIM.H 90

V763 O parâmetro 'coord' é sempre reescrito no corpo da função antes de ser usado. DISPLAY.CPP 4031

void DisplayClass::Set_Tactical_Position(COORDINATE coord)
{
  int xx = 0;
  int yy = 0;

  Confine_Rect(&xx, &yy, TacLeptonWidth, TacLeptonHeight,
    Cell_To_Lepton(MapCellWidth) + GlyphXClientSidebarWidthInLeptons,
    Cell_To_Lepton(MapCellHeight));

  coord = XY_Coord(xx + Cell_To_Lepton(MapCellX), yy + Cell_To_Lepton(....));

  if (ScenarioInit) {
    TacticalCoord = coord;
  }
  DesiredTacticalCoord = coord;
  IsToRedraw = true;
  Flag_To_Redraw(false);
}

O parâmetro coord é substituído imediatamente no corpo da função. O valor antigo não foi usado. É muito suspeito quando uma função possui argumentos e não depende deles. E então existem algumas coordenadas.

E vale a pena conferir este lugar:

V763 O parâmetro 'coord' é sempre reescrito no corpo da função antes de ser usado. DISPLAY.CPP 4251

O ponteiro V507 para a matriz local 'localpalette' é armazenado fora do escopo desta matriz. Esse ponteiro se tornará inválido. MAPSEL.CPP 757

extern "C" unsigned char *InterpolationPalette;

void Map_Selection(void)
{
  unsigned char localpalette[768];
  ....
  InterpolationPalette = localpalette;
  ....
}

Existem muitas variáveis globais no código do jogo. Esta era provavelmente uma abordagem comum para codificação naquela época. Mas agora é considerado ruim e até perigoso.

A paleta local da matriz local é armazenada no ponteiro InterpolationPalette, que se tornará inválido após sair da função.

Mais alguns lugares perigosos:

O ponteiro V507 para a matriz local 'localpalette' está armazenado fora do escopo desta matriz. Esse ponteiro se tornará inválido. MAPSEL.CPP 769
O ponteiro V507 para o 'buffer' da matriz local é armazenado fora do escopo desta matriz. Esse ponteiro se tornará inválido. WINDOWS.CPP 458

Conclusão

Como escrevi no primeiro relatório, vamos torcer para que os novos projetos da Electronic Arts sejam de melhor qualidade. Em geral, os desenvolvedores de jogos estão comprando PVS-Studio ativamente . Agora, os orçamentos dos jogos são bastante grandes, então ninguém precisa dos custos extras para corrigir bugs na produção. E consertar um erro em um estágio inicial de codificação praticamente não exige tempo e outros recursos.

Nós convidamos você a baixar e experimentar o PVS-Studio em todos os projetos em nosso website .

Se você deseja compartilhar este artigo com um público falante de inglês, use o link de tradução: Svyatoslav Razmyslov. O Código do Jogo Command & Conquer: Bugs dos anos 90. Volume dois .

Código do jogo Command & Conquer: bugs dos anos 90. Volume dois