Geekly articles weekly

PVS-Studio visita Apache Hive

Figura 1

Nos últimos dez anos, o movimento de código aberto tem sido um dos principais impulsionadores do desenvolvimento da indústria de TI e seu componente crucial. O papel dos projetos de código aberto está se tornando cada vez mais proeminente, não apenas em termos de quantidade, mas também em termos de qualidade, o que muda o próprio conceito de como eles estão posicionados no mercado de TI em geral. Nossa equipe corajosa do PVS-Studio não está ociosa e participa ativamente do fortalecimento da presença de software de código aberto, encontrando bugs ocultos nas enormes profundidades das bases de código e oferecendo opções de licença gratuitas aos autores de tais projetos. Este artigo é apenas mais uma parte dessa atividade! Hoje vamos falar sobre o Apache Hive. Eu tenho o relatório - e há coisas que vale a pena examinar.

Sobre o PVS-Studio


O analisador de código estático PVS-Studio , que existe há mais de 10 anos, é uma solução de software multifuncional e fácil de integrar. Atualmente, ele suporta C, C ++, C # e Java e é executado no Windows, Linux e macOS.

O PVS-Studio é uma solução B2B paga usada por várias equipes em várias empresas. Se você quiser experimentar o analisador, visite esta página para baixar a distribuição e solicitar uma chave de avaliação.

Se você é um nerd de código aberto ou, digamos, um estudante, pode tirar proveito de uma de nossas opções de licença gratuitas.

Sobre o Apache Hive


A quantidade de dados tem crescido a uma taxa enorme nos últimos anos. Os bancos de dados padrão não podem mais lidar com esse rápido crescimento, que é a origem do termo Big Data, além de outras noções relacionadas (como processamento, armazenamento e outras operações no big data).

Atualmente, acredita-se que o Apache Hadoop seja uma das tecnologias pioneiras de Big Data. Suas principais tarefas são armazenar, processar e gerenciar grandes quantidades de dados. Os principais componentes que compõem a estrutura são o Hadoop Common, HDFS , Hadoop MapReduce e Hadoop YARN . Com o tempo, um grande ecossistema de projetos e tecnologias relacionados se desenvolveu em torno do Hadoop, muitos dos quais começaram originalmente como parte do projeto e começaram a se tornar independentes. O Apache Hive é um deles.

O Apache Hive é um armazém de dados distribuído. Ele gerencia os dados armazenados no HDFS e fornece a linguagem de consulta baseada em SQL (HiveQL) para manipular esses dados. Mais detalhes sobre o projeto podem ser encontrados aqui .

Executando a análise


Não demorou muito esforço ou tempo para iniciar a análise. Aqui está o meu algoritmo:

  • Apache Hive baixado do GitHub ;
  • Leia o guia sobre como iniciar o analisador Java e iniciou a análise;
  • Obteve o relatório do analisador, o estudou e escreveu os casos mais interessantes.

Os resultados da análise são os seguintes: 1456 avisos de níveis alto e médio (602 e 854 respectivamente) em mais de 6500 arquivos.

Nem todos os avisos se referem a erros genuínos. Isso é bastante normal; você precisa ajustar as configurações do analisador antes de começar a usá-lo regularmente. Depois disso, você normalmente espera uma taxa bastante baixa de falsos positivos ( exemplo ).

Deixei de fora os 407 avisos (177 de nível alto e 230 de nível médio) acionados pelos arquivos de teste. Também ignorei o diagnóstico V6022 (já que você não pode distinguir com segurança fragmentos defeituosos e corretos quando não estiver familiarizado com o código), que foi disparado até 482 vezes. Também não examinei os 179 avisos gerados pelo diagnóstico V6021 .

No final, eu ainda tinha avisos suficientes para acompanhar e, como não ajustava as configurações, ainda há uma porcentagem de falsos positivos entre elas. Não faz sentido incluir muitos avisos em um artigo como esse :). Então, apenas falaremos sobre o que chamou minha atenção e pareceu curioso o suficiente.

Condições predeterminadas


Entre os diagnósticos examinados para esta análise, o V6007 mantém um registro para o número de avisos emitidos. Um pouco mais de 200 mensagens !!! Alguns parecem inofensivos, outros suspeitos e outros são bugs genuínos, afinal! Vamos dar uma olhada em alguns deles.

A expressão V6007 'key.startsWith ("hplsql.")' Sempre é verdadeira. Exec.java (675)

void initOptions() { .... if (key == null || value == null || !key.startsWith("hplsql.")) { // <= continue; } else if (key.compareToIgnoreCase(Conf.CONN_DEFAULT) == 0) { .... } else if (key.startsWith("hplsql.conn.init.")) { .... } else if (key.startsWith(Conf.CONN_CONVERT)) { .... } else if (key.startsWith("hplsql.conn.")) { .... } else if (key.startsWith("hplsql.")) { // <= .... } }

Essa é uma construção bastante longa, se-mais-se! O analisador não gostou da condição no último if (key.startsWith ("hplsql.")) Porque se a execução o atingir, isso significará que é verdade. De fato, se você olhar para a primeira linha de toda essa construção if-else-if, verá que ela já contém a verificação oposta; portanto, se a string não começar com "hplsql". , a execução pulará imediatamente para a próxima iteração.

A expressão V6007 'columnNameProperty.length () == 0' é sempre falsa. OrcRecordUpdater.java (238)

 private static TypeDescription getTypeDescriptionFromTableProperties(....) { .... if (tableProperties != null) { final String columnNameProperty = ....; final String columnTypeProperty = ....; if ( !Strings.isNullOrEmpty(columnNameProperty) && !Strings.isNullOrEmpty(columnTypeProperty)) { List<String> columnNames = columnNameProperty.length() == 0 ? new ArrayList<String>() : ....; List<TypeInfo> columnTypes = columnTypeProperty.length() == 0 ? new ArrayList<TypeInfo>() : ....; .... } } } .... }

A comparação do comprimento da string columnNameProperty com zero sempre retornará false . Isso acontece porque essa comparação segue a verificação ! Strings.isNullOrEmpty (columnNameProperty) . Portanto, se a execução atingir nossa condição, isso significa que a string columnNameProperty certamente não é nula nem vazia.

O mesmo vale para a string columnTypeProperty uma linha depois:

  • A expressão V6007 'columnTypeProperty.length () == 0' é sempre falsa. OrcRecordUpdater.java (239)

A expressão V6007 'colOrScalar1.equals ("Column")' é sempre falsa. GenVectorCode.java (3469)

 private void generateDateTimeArithmeticIntervalYearMonth(String[] tdesc) throws Exception { .... String colOrScalar1 = tdesc[4]; .... String colOrScalar2 = tdesc[6]; .... if (colOrScalar1.equals("Col") && colOrScalar1.equals("Column")) // <= { .... } else if (colOrScalar1.equals("Col") && colOrScalar1.equals("Scalar")) { .... } else if (colOrScalar1.equals("Scalar") && colOrScalar1.equals("Column")) { .... } }

O bom e velho copiar e colar. Do ponto de vista da lógica atual, a sequência colOrScalar1 pode ter dois valores diferentes ao mesmo tempo, o que é impossível. Obviamente, as verificações devem ter a variável colOrScalar1 à esquerda e colOrScalar2 à direita.

Avisos semelhantes algumas linhas abaixo:

  • A expressão V6007 'colOrScalar1.equals ("Scalar") "é sempre falsa. GenVectorCode.java (3475)
  • A expressão V6007 'colOrScalar1.equals ("Column")' é sempre falsa. GenVectorCode.java (3486)

Como resultado, essa construção if-else-if nunca fará nada.

Mais alguns avisos do V6007 :

  • V6007 A expressão 'caracteres == nulo' é sempre falsa. RandomTypeUtil.java (43)
  • A expressão V6007 'writeIdHwm> 0' é sempre falsa. TxnHandler.java (1603)
  • A expressão V6007 'fields.equals ("*")' sempre é verdadeira. Server.java (983)
  • A expressão V6007 'currentGroups! = Null' sempre é verdadeira. GenericUDFCurrentGroups.java (90)
  • A expressão V6007 'this.wh == null' é sempre falsa. Novo retorna referência não nula. StorageBasedAuthorizationProvider.java (93), StorageBasedAuthorizationProvider.java (92)
  • e assim por diante ...

NPE


V6008 Dereferência nula potencial de 'dagLock'. QueryTracker.java (557), QueryTracker.java (553)

 private void handleFragmentCompleteExternalQuery(QueryInfo queryInfo) { if (queryInfo.isExternalQuery()) { ReadWriteLock dagLock = getDagLock(queryInfo.getQueryIdentifier()); if (dagLock == null) { LOG.warn("Ignoring fragment completion for unknown query: {}", queryInfo.getQueryIdentifier()); } boolean locked = dagLock.writeLock().tryLock(); ..... } }

Um objeto nulo é capturado, registrado e ... o programa continua em execução. Como resultado, a verificação é seguida por uma dereferência de ponteiro nulo. Ai!

Os desenvolvedores realmente queriam que o programa saísse da função ou lançasse alguma exceção especial no caso de obter uma referência nula.

V6008 Dereferência nula de 'buffer' na função 'unlockSingleBuffer'. MetadataCache.java (410), MetadataCache.java (465)

 private boolean lockBuffer(LlapBufferOrBuffers buffers, ....) { LlapAllocatorBuffer buffer = buffers.getSingleLlapBuffer(); if (buffer != null) { // <= return lockOneBuffer(buffer, doNotifyPolicy); } LlapAllocatorBuffer[] bufferArray = buffers.getMultipleLlapBuffers(); for (int i = 0; i < bufferArray.length; ++i) { if (lockOneBuffer(bufferArray[i], doNotifyPolicy)) continue; for (int j = 0; j < i; ++j) { unlockSingleBuffer(buffer, true); // <= } .... } .... } .... private void unlockSingleBuffer(LlapAllocatorBuffer buffer, ....) { boolean isLastDecref = (buffer.decRef() == 0); // <= if (isLastDecref) { .... } }

Outro potencial NPE. Se a execução atingir o método unlockSingleBuffer , isso significa que o objeto de buffer é nulo. Suponha que foi isso que aconteceu! Se você observar o método unlockSingleBuffer , notará como nosso objeto é desreferenciado na primeira linha. Gotcha!

Uma mudança enlouquecida


V6034 A alteração no valor de 'bitShiftsInWord - 1' pode ser inconsistente com o tamanho do tipo: 'bitShiftsInWord - 1' = [-1 ... 30]. UnsignedInt128.java (1791)

 private void shiftRightDestructive(int wordShifts, int bitShiftsInWord, boolean roundUp) { if (wordShifts == 0 && bitShiftsInWord == 0) { return; } assert (wordShifts >= 0); assert (bitShiftsInWord >= 0); assert (bitShiftsInWord < 32); if (wordShifts >= 4) { zeroClear(); return; } final int shiftRestore = 32 - bitShiftsInWord; // check this because "123 << 32" will be 123. final boolean noRestore = bitShiftsInWord == 0; final int roundCarryNoRestoreMask = 1 << 31; final int roundCarryMask = (1 << (bitShiftsInWord - 1)); // <= .... }

Essa é uma mudança potencial em -1. Se o método for chamado com, digamos, wordShifts == 3 e bitShiftsInWord == 0 , a linha relatada terminará com 1 << -1. Esse é um comportamento planejado?

V6034 A mudança no valor de 'j' pode ser inconsistente com o tamanho do tipo: 'j' = [0 ... 63]. IoTrace.java (272)

 public void logSargResult(int stripeIx, boolean[] rgsToRead) { .... for (int i = 0, valOffset = 0; i < elements; ++i, valOffset += 64) { long val = 0; for (int j = 0; j < 64; ++j) { int ix = valOffset + j; if (rgsToRead.length == ix) break; if (!rgsToRead[ix]) continue; val = val | (1 << j); // <= } .... } .... }

Na linha reportada, a variável j pode ter um valor dentro do intervalo [0 ... 63]. Por esse motivo, o cálculo do valor de val no loop pode ser executado de maneira inesperada. Na expressão (1 << j) , o valor 1 é do tipo int , portanto, deslocá-lo em 32 bits e mais nos leva além dos limites do intervalo do tipo. Isso pode ser corrigido escrevendo ((long) 1 << j) .

Se deixando levar pelo log


V6046 Formato incorreto. É esperado um número diferente de itens de formato. Argumentos não utilizados: 1, 2. StatsSources.java (89)

 private static ImmutableList<PersistedRuntimeStats> extractStatsFromPlanMapper (....) { .... if (stat.size() > 1 || sig.size() > 1) { StringBuffer sb = new StringBuffer(); sb.append(String.format( "expected(stat-sig) 1-1, got {}-{} ;", // <= stat.size(), sig.size() )); .... } .... if (e.getAll(OperatorStats.IncorrectRuntimeStatsMarker.class).size() > 0) { LOG.debug( "Ignoring {}, marked with OperatorStats.IncorrectRuntimeStatsMarker", sig.get(0) ); continue; } .... }

Ao escrever o código para formatar a string usando String.format () , o desenvolvedor usou sintaxe incorreta. Como resultado, os parâmetros transmitidos nunca chegaram à sequência resultante. Meu palpite é que o desenvolvedor estava trabalhando no log antes de escrever isso, que é de onde eles pegaram emprestada a sintaxe.

Uma exceção roubada


V6051 O uso da instrução 'return' no bloco 'final' pode levar à perda de exceções não tratadas. ObjectStore.java (9080)

 private List<MPartitionColumnStatistics> getMPartitionColumnStatistics(....) throws NoSuchObjectException, MetaException { boolean committed = false; try { .... /*some actions*/ committed = commitTransaction(); return result; } catch (Exception ex) { LOG.error("Error retrieving statistics via jdo", ex); if (ex instanceof MetaException) { throw (MetaException) ex; } throw new MetaException(ex.getMessage()); } finally { if (!committed) { rollbackTransaction(); return Lists.newArrayList(); } } }

Retornar qualquer coisa do bloco final é uma prática muito ruim, e este exemplo mostra claramente o porquê.

No bloco try , o programa está formando uma solicitação e acessando o armazenamento. A variável confirmada possui o valor false por padrão e altera seu estado somente depois que todas as ações anteriores no bloco try foram executadas com êxito. Isso significa que, se uma exceção for gerada, essa variável sempre será falsa . O bloco catch irá capturar a exceção, ajustá-lo um pouco e ativá-lo. Portanto, quando for a vez do bloco final , a execução entrará na condição da qual uma lista vazia será retornada. Quanto custa esse retorno? Bem, custa-nos impedir que qualquer exceção capturada seja lançada para o exterior, onde possa ser tratada adequadamente. Nenhuma das exceções especificadas na assinatura do método será lançada; eles são simplesmente enganosos.

Uma mensagem de diagnóstico semelhante:

  • V6051 O uso da instrução 'return' no bloco 'final' pode levar à perda de exceções não tratadas. ObjectStore.java (808)

Diversos


V6009 A função 'compareTo' recebe um argumento estranho. Um objeto 'o2.getWorkerIdentity ()' é usado como argumento para seu próprio método. LlapFixedRegistryImpl.java (244)

 @Override public List<LlapServiceInstance> getAllInstancesOrdered(....) { .... Collections.sort(list, new Comparator<LlapServiceInstance>() { @Override public int compare(LlapServiceInstance o1, LlapServiceInstance o2) { return o2.getWorkerIdentity().compareTo(o2.getWorkerIdentity()); // <= } }); .... }

Pode haver várias causas que levam a um erro tão tolo: copiar e colar, descuido, pressa e assim por diante. Muitas vezes vemos erros como esse em projetos de código aberto e até temos um artigo inteiro sobre isso.

V6020 Divida por zero. O intervalo dos valores do denominador 'divisor' inclui zero. SqlMathUtil.java (265)

 public static long divideUnsignedLong(long dividend, long divisor) { if (divisor < 0L) { /*some comments*/ return (compareUnsignedLong(dividend, divisor)) < 0 ? 0L : 1L; } if (dividend >= 0) { // Both inputs non-negative return dividend / divisor; // <= } else { .... } }

Este é bastante trivial. Uma série de verificações foi impotente para evitar a divisão por zero.

Mais alguns avisos:

  • V6020 Mod por zero. O intervalo dos valores do denominador 'divisor' inclui zero. SqlMathUtil.java (309)
  • V6020 Divida por zero. O intervalo dos valores do denominador 'divisor' inclui zero. SqlMathUtil.java (276)
  • V6020 Divida por zero. O intervalo dos valores do denominador 'divisor' inclui zero. SqlMathUtil.java (312)

V6030 O método localizado à direita do '|' O operador será chamado independentemente do valor do operando esquerdo. Talvez seja melhor usar '||'. OperatorUtils.java (573)

 public static Operator<? extends OperatorDesc> findSourceRS(....) { .... List<Operator<? extends OperatorDesc>> parents = ....; if (parents == null | parents.isEmpty()) { // reached end eg TS operator return null; } .... }

O programador escreveu o operador bit a bit | em vez da lógica ||. Isso significa que a parte direita será executada, independentemente do resultado da esquerda. Se pais == nulo , esse erro de digitação acabará com um NPE na próxima subexpressão lógica.

V6042 A expressão é verificada quanto à compatibilidade com o tipo 'A', mas é convertida para o tipo 'B'. VectorColumnAssignFactory.java (347)

 public static VectorColumnAssign buildObjectAssign(VectorizedRowBatch outputBatch, int outColIndex, PrimitiveCategory category) throws HiveException { VectorColumnAssign outVCA = null; ColumnVector destCol = outputBatch.cols[outColIndex]; if (destCol == null) { .... } else if (destCol instanceof LongColumnVector) { switch(category) { .... case LONG: outVCA = new VectorLongColumnAssign() { .... } .init(.... , (LongColumnVector) destCol); break; case TIMESTAMP: outVCA = new VectorTimestampColumnAssign() { .... }.init(...., (TimestampColumnVector) destCol); // <= break; case DATE: outVCA = new VectorLongColumnAssign() { .... } .init(...., (LongColumnVector) destCol); break; case INTERVAL_YEAR_MONTH: outVCA = new VectorLongColumnAssign() { .... }.init(...., (LongColumnVector) destCol); break; case INTERVAL_DAY_TIME: outVCA = new VectorIntervalDayTimeColumnAssign() { .... }.init(...., (IntervalDayTimeColumnVector) destCol);// <= break; default: throw new HiveException(....); } } else if (destCol instanceof DoubleColumnVector) { .... } .... else { throw new HiveException(....); } return outVCA; }

Estamos interessados ​​nas classes LongColumnVector estende ColumnVector e TimestampColumnVector estende ColumnVector . A verificação de que o objeto destCol é uma instância do LongColumnVector sugere explicitamente que é um objeto dessa classe que será tratado no corpo da instrução condicional. Apesar disso, no entanto, ele ainda é transmitido para TimestampColumnVector ! Como você pode ver, essas são classes diferentes, exceto que são derivadas do mesmo pai. Como resultado, obtemos uma ClassCastException .

O mesmo acontece no caso de transmissão para IntervalDayTimeColumnVector :

  • V6042 A expressão é verificada quanto à compatibilidade com o tipo 'A', mas é convertida para o tipo 'B'. VectorColumnAssignFactory.java (390)

V6060 A referência 'var' foi utilizada antes de ser verificada com relação a nulo. Var.java (402), Var.java (395)

 @Override public boolean equals(Object obj) { if (getClass() != obj.getClass()) { // <= return false; } Var var = (Var)obj; if (this == var) { return true; } else if (var == null || // <= var.value == null || this.value == null) { return false; } .... }

Aqui você vê uma verificação estranha do objeto var para null depois que a desreferência já ocorreu. Nesse contexto, var e obj são o mesmo objeto ( var = (Var) obj ). A presença da verificação nula implica que o objeto passado pode ser nulo. Portanto, chamar igual (nulo) resultará em um NPE, em vez do falso esperado, logo na primeira linha. Sim, o cheque está lá, mas, infelizmente, está no lugar errado.

Alguns outros casos semelhantes, em que um objeto é usado antes da verificação:

  • V6060 A referência 'value' foi utilizada antes de ser verificada em relação a null. ParquetRecordReaderWrapper.java (168), ParquetRecordReaderWrapper.java (166)
  • V6060 A referência 'defaultConstraintCols' foi utilizada antes de ser verificada com relação a nulo. HiveMetaStore.java (2539), HiveMetaStore.java (2530)
  • V6060 A referência 'projIndxLst' foi utilizada antes de ser verificada como nula. RelOptHiveTable.java (683), RelOptHiveTable.java (682)
  • V6060 A referência 'oldp' foi utilizada antes de ser verificada com relação a nulo. ObjectStore.java (4343), ObjectStore.java (4339)
  • e assim por diante ...

Conclusão


Se você alguma vez se interessou por Big Data, mesmo que apenas um pouco, dificilmente poderia desconhecer a importância do Apache Hive. Este é um projeto popular e bastante amplo, composto por mais de 6500 arquivos de origem (* .java). Muitos desenvolvedores o escrevem há muitos anos, o que significa que há muitas coisas para um analisador estático encontrar lá. Apenas prova mais uma vez que a análise estática é extremamente importante e útil no desenvolvimento de projetos de médio e grande porte!

Nota Verificações únicas como a que eu fiz aqui são boas para mostrar as capacidades do analisador, mas são um cenário totalmente inadequado de usá-lo. Essa idéia é elaborada aqui e aqui . A análise estática deve ser usada regularmente!

Essa verificação do Hive revelou muitos defeitos e fragmentos suspeitos. Se os autores do Apache Hive se depararem com este artigo, teremos prazer em ajudar com o trabalho duro de melhorar o projeto.

Você não pode imaginar o Apache Hive sem o Apache Hadoop; portanto, o Unicorn do PVS-Studio também pode fazer uma visita a ele. Mas isso é tudo por hoje. Enquanto isso, convido você a baixar o analisador e verificar seus próprios projetos.

Source: https://habr.com/ru/post/pt463753/

More articles:


All Articles