Chromium: other errors
Go 
We are completing a series of articles on recommendations for writing high-quality code using the example of errors found in the Chromium project. After writing 6 articles, we still have a lot of unexamined errors. These errors are heterogeneous and it is difficult to compile information about them on a particular topic. Therefore, in this seventh article, the most interesting defects will be simply selected and considered.
As I wrote in the introductory article , looking through the report issued by the PVS-Studio analyzer, I noticed about 250 errors in the Chromium project and the libraries used in it. I looked at the report quite fluently, so, in fact, you can find a lot more mistakes.
After the introductory article, I wrote another 6, where I looked at various patterns of errors. I wrote these articles, wrote, and still, I have about 70 examples of errors. Based on the remaining bugs, I find it difficult to make articles on various topics. I may be tired. However, there is another reason - I’m waiting for the XNU verification report and I don’t wait for them to work out.
Therefore, I decided to complete a series of articles about Chromium and write this latest article, where I will consider the most interesting of the remaining errors. I remind you that the full list of errors is given here: chromium.txt .
')
Chromium project.
PVS-Studio warning : V522 CWE-476 Dereferencing of the null pointer 'context' might take place. device_media_async_file_util.cc 322
Depending on the luck, and more precisely on the compiler used, this code can both work correctly and lead to the dereferencing of the null pointer.
Whether dereferencing a null pointer or not depends on the order in which the arguments are calculated when the CreateSnapshotFile function is called . In C ++, the order of evaluation of the function's arguments is unspecified (unspecified behavior). If the std :: move (context) argument is calculated first, then the context-> task_runner () will dereference the null pointer.
Recommendation. Do not try to cram as many actions as possible in one line - this often fails. Write a simple code, and it is more likely to get the right one.
Chromium project.
PVS-Studio warning : V708 CWE-758 Dangerous construction is used: 'm [x] = m.size ()', where 'm' is of 'unordered_map' class. This may lead to undefined behavior. trace_log.cc 1343
This code is so complex that it is even incomprehensible whether there is now indefinite behavior in it or not. The fact is that the C ++ standard is changing and some constructions that previously led to undefined behavior become correct. Let me explain this with simple examples:
Let's return to the code from the Chromium project. The expression thread_colors_ [thread_name] may or may not create a new element in the container and return a reference to an already existing element. The main thing is that thread_colors_ [thread_name] can change the number of elements in an associative container.
The expression (thread_colors_.size ()% 6) + 1 depends on the number of elements in the thread_colors_ associative container.
Different values will be obtained depending on whether the expression to the left of the assignment operator = or the expression to the right is evaluated first.
What determines the order of calculation? From the version of the language used. In any case, do not write so. This code is very difficult to understand.
Recommendation. The advice is the same: do not try to cram as many actions as possible into one line.
ICU library.
PVS-Studio warning : V694 CWE-571 The condition (action + 1) is only false if there is a pointer overflow which is undefined behavior anyway. ubiditransform.cpp 502
The condition is always true. Theoretically, it can become false if an overflow occurs, but this leads to an undefined behavior.
WebRTC library.
PVS-Studio warning : V789 CWE-672 iterators stereocodecfactory.cc 89
The analyzer detected an iterator in the range-based for loop. The above code is similar to the following:
Now you can see that when the push_back function is called, invalidation of the format and __end iterators can occur if memory reallocation occurs within the vector.
Recommendation. Remember that in range-based for loops it is impossible, as in loops organized with the help of iterators, to change the number of elements in a container.
Chromium project.
PVS-Studio Warning: V649 CWE-561 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains function return. This means that the statement is senseless. Check lines: 67, 71. tsf_input_scope.cc 71
It makes no sense to re-check the pointer input_scopes , since if it is zero, the check at the beginning of the function will work, and the function will return the status E_INVALIDARG .
The error is that in the " if (! Input_scopes) " forgot the * operator. Because of this, the pointer that the CoTaskMemAlloc function returns is not checked. The code should be:
Skia Library.
PVS-Studio analyzer considers this code suspicious immediately for two reasons:
Very suspicious code, but it’s hard for me to understand how it should work and what exactly the error here is. Those interested can try to figure out how the undoneSpan function should look like.
WebKit library.
PVS-Studio Warning: V547 CWE-570 Expression '! First' is always always false. webmediaconstraints.cpp 302
Since the variable first is always true , commas between elements will not be added. The correct code is:
ICU library.
PVS-Studio warning : V779 CWE-561 Unreachable code detected. It is possible that an error is present. collationdatabuilder.cpp 392
A loop can only be interrupted by calling the return statement . This means that the assignment after the loop will never be executed.
Ced Library
PVS-Studio warning : V751 Parameter 'byte1' is not used inside function body. compact_enc_det.cc 2559
It is very suspicious that the argument byte1 is not used in the function. I do not know if it is a mistake or not. Even if not an error, it is better not to write like that. Such code will puzzle both those who will accompany it and various analyzers.
Sometimes programmers mistakenly imagine how certain functions or constructions of a language work. Let's look at some such errors.
Chromium project.
PVS-Studio warning: V530 CWE-252 The return value of the function 'remove_if' is required to be utilized. pdf_to_emf_converter.cc 44
The remove_if function does n’t delete anything, it only swaps the elements in the container. Most likely, it should be written here:
V8 engine.
PVS-Studio warning: V547 CWE-570 Expression 'inf == - 1' is always false. string-stream.cc 149
Description of the function std :: isinf : isinf .
As you can see, the std :: isinf function returns the bool type. Thus, this function is clearly used incorrectly.
Skia Library.
PVS-Studio warning : V554 CWE-762 Incorrect use of unique_ptr. The memory is allocated with 'new []' will be cleaned using 'delete'. grglprogrambuilder.cpp 272
Memory is allocated using the new [] operator, and will be freed using the delete operator. The class unique_ptr needs to be told how to manage memory.
The correct code is:
Another similar blunder in the Skia library:
PVS-Studio warning: V554 CWE-762 Incorrect use of unique_ptr. The memory allocated with 'malloc' will be cleaned using 'delete'. grglprogrambuilder.cpp 275
Obviously, one of the programmers found out about the existence of the class std :: unique_ptr , but cannot use it yet :). Memory is allocated using the malloc function, and freed using the delete operator.
The correct code is:
WebKit library.
PVS-Studio Warning: V603 CWE-665 If you wish to call the constructor, 'this-> ScrollAnchorData :: ScrollAnchorData (....)' should be used. webscrollanchordata.h 49
This is not a call to one constructor from another constructor. An unnamed object is simply created and immediately destroyed.
Very often, programs incorrectly written to check whether the pointer is zero or not. Errors of this type can be divided into two main options:
The first option is when the pointer is first dereferenced, and only then checked:
The second option is when first checking the pointer before using it, and then forgetting to do it:
Errors of the first type are detected by the V595 diagnostics, and those of the second type are diagnosed by the V1004 .
Such errors do not always lead to problems. First, it happens that the pointer never becomes zero. In this case, there is no error at all, but there is only an extra check, which confuses both people reading the code and code analyzers. Secondly, it happens that the pointer becomes zero in extremely rare cases, and therefore the error does not affect the typical scenarios of the program.
Nevertheless, messages V595 and V1004 deserve close attention and editing code. The PVS-Studio analyzer issued a bunch of such messages for Chromium code and libraries. Unfortunately, as I described in the introductory article, due to the DCHECK macro , most of them are false positives. Therefore, I was very quickly tired of viewing these messages. A closer look at the messages V595 and V1004 should be after configuring the analyzer.
In any case, I assure you that the errors associated with incorrect verification of pointers are complete. Some of them you can find by looking into the file chromium.txt . To find the rest, you need heroes who set up the analyzer and learn a new report.
I will not give here all the noticed errors, as they are of the same type. I will show only two examples for each diagnostics, so that it is completely clear what kind of errors are being discussed.
V595, the first example, the project Chromium.
PVS-Studio warning: V595 CWE-476 Check lines: 124, 126. paint_op_reader.cc 124
The val pointer is dereferenceed without checking for equality nullptr .
V595, the second example, the Chromium project.
PVS-Studio warning: V595 CWE-476 Check lines: 122, 124. http_auth_handler_factory.cc 122
The factory pointer is dereferenceed without checking for nullptr equality.
V1004, the first example, is a PDFium library.
PVS-Studio warning: V1004 CWE-476 The 'pObject2Device' pointer was used unsafely after it was verified against nullptr. Check lines: 237, 248. cfx_psrenderer.cpp 248
The pObject2Device pointer may be null, as evidenced by checking this pointer for nullptr equality. However, further the pointer is dereferenced without prior verification.
V1004, the second example, is the SwiftShader library.
PVS-Studio warning: V1004 CWE-476 The 'shader' pointer was used unsafely after it was verified against nullptr. Check lines: 43, 53. vertexprogram.cpp 53
The shader pointer can be null, as evidenced by checking this pointer for nullptr equality. However, further the pointer is dereferenced without prior verification.
The PVS-Studio team sends its greetings to Google’s developers and is ready to cooperate. It is possible to discuss at least two options:
Try PVS-Studio. Write to us . We will help with project verification and issue a temporary license for detailed information.
Thanks to everyone who got to the end of the series of articles. I hope you were interested.
As you can see, even in such a high-quality project as Chromium there are a lot of errors that the PVS-Studio analyzer is able to detect. I suggest you start using it in your projects.
If you want to share this article with an English-speaking audience, then please use the link to the translation: Andrey Karpov. Chromium: Miscellaneous Defects .
We are completing a series of articles on recommendations for writing high-quality code using the example of errors found in the Chromium project. After writing 6 articles, we still have a lot of unexamined errors. These errors are heterogeneous and it is difficult to compile information about them on a particular topic. Therefore, in this seventh article, the most interesting defects will be simply selected and considered.As I wrote in the introductory article , looking through the report issued by the PVS-Studio analyzer, I noticed about 250 errors in the Chromium project and the libraries used in it. I looked at the report quite fluently, so, in fact, you can find a lot more mistakes.
After the introductory article, I wrote another 6, where I looked at various patterns of errors. I wrote these articles, wrote, and still, I have about 70 examples of errors. Based on the remaining bugs, I find it difficult to make articles on various topics. I may be tired. However, there is another reason - I’m waiting for the XNU verification report and I don’t wait for them to work out.
Therefore, I decided to complete a series of articles about Chromium and write this latest article, where I will consider the most interesting of the remaining errors. I remind you that the full list of errors is given here: chromium.txt .
')
Undefined and unspecified behavior
Chromium project.
void DeviceMediaAsyncFileUtil::CreateOrOpen( std::unique_ptr<FileSystemOperationContext> context, ....) { .... CreateSnapshotFile( std::move(context), url, base::Bind( &NativeMediaFileUtil::CreatedSnapshotFileForCreateOrOpen, base::RetainedRef(context->task_runner()), file_flags, callback)); } PVS-Studio warning : V522 CWE-476 Dereferencing of the null pointer 'context' might take place. device_media_async_file_util.cc 322
Depending on the luck, and more precisely on the compiler used, this code can both work correctly and lead to the dereferencing of the null pointer.
Whether dereferencing a null pointer or not depends on the order in which the arguments are calculated when the CreateSnapshotFile function is called . In C ++, the order of evaluation of the function's arguments is unspecified (unspecified behavior). If the std :: move (context) argument is calculated first, then the context-> task_runner () will dereference the null pointer.
Recommendation. Do not try to cram as many actions as possible in one line - this often fails. Write a simple code, and it is more likely to get the right one.
Chromium project.
std::unordered_map<std::string, int> thread_colors_; std::string TraceLog::EventToConsoleMessage(....) { .... thread_colors_[thread_name] = (thread_colors_.size() % 6) + 1; .... } PVS-Studio warning : V708 CWE-758 Dangerous construction is used: 'm [x] = m.size ()', where 'm' is of 'unordered_map' class. This may lead to undefined behavior. trace_log.cc 1343
This code is so complex that it is even incomprehensible whether there is now indefinite behavior in it or not. The fact is that the C ++ standard is changing and some constructions that previously led to undefined behavior become correct. Let me explain this with simple examples:
i = ++i + 2; // undefined behavior until C++11 i = i++ + 2; // undefined behavior until C++17 f(i = -2, i = -2); // undefined behavior until C++17 f(++i, ++i); // undefined behavior until C++17, // unspecified after C++17 i = ++i + i++; // undefined behavior cout << i << i++; // undefined behavior until C++17 a[i] = i++; // undefined behavior until C++17 n = ++i + i; // undefined behavior Let's return to the code from the Chromium project. The expression thread_colors_ [thread_name] may or may not create a new element in the container and return a reference to an already existing element. The main thing is that thread_colors_ [thread_name] can change the number of elements in an associative container.
The expression (thread_colors_.size ()% 6) + 1 depends on the number of elements in the thread_colors_ associative container.
Different values will be obtained depending on whether the expression to the left of the assignment operator = or the expression to the right is evaluated first.
What determines the order of calculation? From the version of the language used. In any case, do not write so. This code is very difficult to understand.
Recommendation. The advice is the same: do not try to cram as many actions as possible into one line.
ICU library.
U_DRAFT uint32_t U_EXPORT2 ubiditransform_transform(....) { .... const UBiDiAction *action = NULL; .... if (action + 1) { updateSrc(....); } .... } PVS-Studio warning : V694 CWE-571 The condition (action + 1) is only false if there is a pointer overflow which is undefined behavior anyway. ubiditransform.cpp 502
The condition is always true. Theoretically, it can become false if an overflow occurs, but this leads to an undefined behavior.
WebRTC library.
std::vector<SdpVideoFormat> StereoDecoderFactory::GetSupportedFormats() const { std::vector<SdpVideoFormat> formats = ....; for (const auto& format : formats) { // <= if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); // <= } } return formats; } PVS-Studio warning : V789 CWE-672 iterators stereocodecfactory.cc 89
The analyzer detected an iterator in the range-based for loop. The above code is similar to the following:
for (auto format = begin(formats), __end = end(formats); format != __end; ++format) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } } Now you can see that when the push_back function is called, invalidation of the format and __end iterators can occur if memory reallocation occurs within the vector.
Recommendation. Remember that in range-based for loops it is impossible, as in loops organized with the help of iterators, to change the number of elements in a container.
Errors in logic
Chromium project.
STDMETHOD(GetInputScopes)(InputScope** input_scopes, UINT* count) override { if (!count || !input_scopes) return E_INVALIDARG; *input_scopes = static_cast<InputScope*>(CoTaskMemAlloc( sizeof(InputScope) * input_scopes_.size())); if (!input_scopes) { *count = 0; return E_OUTOFMEMORY; } .... } PVS-Studio Warning: V649 CWE-561 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains function return. This means that the statement is senseless. Check lines: 67, 71. tsf_input_scope.cc 71
It makes no sense to re-check the pointer input_scopes , since if it is zero, the check at the beginning of the function will work, and the function will return the status E_INVALIDARG .
The error is that in the " if (! Input_scopes) " forgot the * operator. Because of this, the pointer that the CoTaskMemAlloc function returns is not checked. The code should be:
*input_scopes = static_cast<InputScope*>(CoTaskMemAlloc( sizeof(InputScope) * input_scopes_.size())); if (!*input_scopes) { *count = 0; return E_OUTOFMEMORY; } Skia Library.
SkOpSpan* SkOpContour::undoneSpan() { SkOpSegment* testSegment = &fHead; bool allDone = true; do { if (testSegment->done()) { continue; } allDone = false; return testSegment->undoneSpan(); } while ((testSegment = testSegment->next())); if (allDone) { fDone = true; } return nullptr; } PVS-Studio analyzer considers this code suspicious immediately for two reasons:
- V547 CWE-571 Expression 'allDone' is always true. skopcontour.cpp 43
- V1001 CWE-563 The "AllDone" variable is assigned. skopcontour.cpp 40
Very suspicious code, but it’s hard for me to understand how it should work and what exactly the error here is. Those interested can try to figure out how the undoneSpan function should look like.
WebKit library.
WebString StringConstraint::ToString() const { .... bool first = true; for (const auto& iter : exact_) { if (!first) builder.Append(", "); builder.Append('"'); builder.Append(iter); builder.Append('"'); } .... } PVS-Studio Warning: V547 CWE-570 Expression '! First' is always always false. webmediaconstraints.cpp 302
Since the variable first is always true , commas between elements will not be added. The correct code is:
bool first = true; for (const auto& iter : exact_) { if (first) first = false; else builder.Append(", "); builder.Append('"'); builder.Append(iter); builder.Append('"'); } ICU library.
uint32_t CollationDataBuilder::setPrimaryRangeAndReturnNext(....) { .... } else { // Short range: Set individual CE32s. for(;;) { utrie2_set32(....); ++start; primary = Collation::incThreeBytePrimaryByOffset(....); if(start > end) { return primary; } } modified = TRUE; // <= } } PVS-Studio warning : V779 CWE-561 Unreachable code detected. It is possible that an error is present. collationdatabuilder.cpp 392
A loop can only be interrupted by calling the return statement . This means that the assignment after the loop will never be executed.
Ced Library
void HzBoostWhack(DetectEncodingState* destatep, uint8 byte1, uint8 byte2) { if ((byte2 == '{') || (byte2 == '}')) { // Found ~{ or ~} Boost(destatep, F_HZ_GB_2312, kBoostOnePair); } else if ((byte2 == '~') || (byte2 == '\n')) { // neutral destatep->enc_prob[F_HZ_GB_2312] += 0; } else { // Illegal pair Whack(destatep, F_HZ_GB_2312, kBadPairWhack); } } PVS-Studio warning : V751 Parameter 'byte1' is not used inside function body. compact_enc_det.cc 2559
It is very suspicious that the argument byte1 is not used in the function. I do not know if it is a mistake or not. Even if not an error, it is better not to write like that. Such code will puzzle both those who will accompany it and various analyzers.
Wrong expectations
Sometimes programmers mistakenly imagine how certain functions or constructions of a language work. Let's look at some such errors.
Chromium project.
void OnConvertedClientDisconnected() { // We have no direct way of tracking which // PdfToEmfConverterClientPtr got disconnected as it is a // movable type, short of using a wrapper. // Just traverse the list of clients and remove the ones // that are not bound. std::remove_if( g_converter_clients.Get().begin(), g_converter_clients.Get().end(), [](const mojom::PdfToEmfConverterClientPtr& client) { return !client.is_bound(); }); } PVS-Studio warning: V530 CWE-252 The return value of the function 'remove_if' is required to be utilized. pdf_to_emf_converter.cc 44
The remove_if function does n’t delete anything, it only swaps the elements in the container. Most likely, it should be written here:
auto trash = std::remove_if(........); g_converter_clients.Get().erase(trash, g_converter_clients.Get().end()); V8 engine.
void StringStream::Add(....) { .... case 'f': case 'g': case 'G': case 'e': case 'E': { double value = current.data_.u_double_; int inf = std::isinf(value); if (inf == -1) { Add("-inf"); } else if (inf == 1) { Add("inf"); } else if (std::isnan(value)) { Add("nan"); } else { EmbeddedVector<char, 28> formatted; SNPrintF(formatted, temp.start(), value); Add(formatted.start()); } break; } .... } PVS-Studio warning: V547 CWE-570 Expression 'inf == - 1' is always false. string-stream.cc 149
Description of the function std :: isinf : isinf .
As you can see, the std :: isinf function returns the bool type. Thus, this function is clearly used incorrectly.
Skia Library.
GrGLProgram* GrGLProgramBuilder::finalize() { .... std::unique_ptr<char> binary(new char[length]); .... } PVS-Studio warning : V554 CWE-762 Incorrect use of unique_ptr. The memory is allocated with 'new []' will be cleaned using 'delete'. grglprogrambuilder.cpp 272
Memory is allocated using the new [] operator, and will be freed using the delete operator. The class unique_ptr needs to be told how to manage memory.
The correct code is:
std::unique_ptr<char[]> binary(new char[length]); Another similar blunder in the Skia library:
GrGLProgram* GrGLProgramBuilder::finalize() { .... std::unique_ptr<uint8_t> data((uint8_t*) malloc(dataLength)); .... } PVS-Studio warning: V554 CWE-762 Incorrect use of unique_ptr. The memory allocated with 'malloc' will be cleaned using 'delete'. grglprogrambuilder.cpp 275
Obviously, one of the programmers found out about the existence of the class std :: unique_ptr , but cannot use it yet :). Memory is allocated using the malloc function, and freed using the delete operator.
The correct code is:
std::unique_ptr<uint8_t, void (*)(void*)> data((uint8_t*) malloc(dataLength), std::free); WebKit library.
struct ScrollAnchorData { WebString selector_; WebFloatPoint offset_; uint64_t simhash_; ScrollAnchorData(const WebString& selector, const WebFloatPoint& offset, uint64_t simhash) : selector_(selector), offset_(offset), simhash_(simhash) {} ScrollAnchorData() { ScrollAnchorData(WebString(), WebFloatPoint(0, 0), 0); } }; PVS-Studio Warning: V603 CWE-665 If you wish to call the constructor, 'this-> ScrollAnchorData :: ScrollAnchorData (....)' should be used. webscrollanchordata.h 49
This is not a call to one constructor from another constructor. An unnamed object is simply created and immediately destroyed.
Poor pointer checks
Very often, programs incorrectly written to check whether the pointer is zero or not. Errors of this type can be divided into two main options:
The first option is when the pointer is first dereferenced, and only then checked:
p[n] = 1; if (!p) return false; The second option is when first checking the pointer before using it, and then forgetting to do it:
if (p) p[0] = x; p[1] = y; Errors of the first type are detected by the V595 diagnostics, and those of the second type are diagnosed by the V1004 .
Such errors do not always lead to problems. First, it happens that the pointer never becomes zero. In this case, there is no error at all, but there is only an extra check, which confuses both people reading the code and code analyzers. Secondly, it happens that the pointer becomes zero in extremely rare cases, and therefore the error does not affect the typical scenarios of the program.
Nevertheless, messages V595 and V1004 deserve close attention and editing code. The PVS-Studio analyzer issued a bunch of such messages for Chromium code and libraries. Unfortunately, as I described in the introductory article, due to the DCHECK macro , most of them are false positives. Therefore, I was very quickly tired of viewing these messages. A closer look at the messages V595 and V1004 should be after configuring the analyzer.
In any case, I assure you that the errors associated with incorrect verification of pointers are complete. Some of them you can find by looking into the file chromium.txt . To find the rest, you need heroes who set up the analyzer and learn a new report.
I will not give here all the noticed errors, as they are of the same type. I will show only two examples for each diagnostics, so that it is completely clear what kind of errors are being discussed.
V595, the first example, the project Chromium.
template <typename T> void PaintOpReader::ReadFlattenable(sk_sp<T>* val) { // .... // val . // .... val->reset(static_cast<T*>(SkValidatingDeserializeFlattenable( const_cast<const char*>(memory_), bytes, T::GetFlattenableType()))); if (!val) SetInvalid(); .... } PVS-Studio warning: V595 CWE-476 Check lines: 124, 126. paint_op_reader.cc 124
The val pointer is dereferenceed without checking for equality nullptr .
V595, the second example, the Chromium project.
void HttpAuthHandlerRegistryFactory::RegisterSchemeFactory( const std::string& scheme, HttpAuthHandlerFactory* factory) { factory->set_http_auth_preferences(http_auth_preferences()); std::string lower_scheme = base::ToLowerASCII(scheme); if (factory) factory_map_[lower_scheme] = base::WrapUnique(factory); else factory_map_.erase(lower_scheme); } PVS-Studio warning: V595 CWE-476 Check lines: 122, 124. http_auth_handler_factory.cc 122
The factory pointer is dereferenceed without checking for nullptr equality.
V1004, the first example, is a PDFium library.
void CFX_PSRenderer::SetClip_PathStroke(...., const CFX_Matrix* pObject2Device, ....) { .... if (pObject2Device) { .... } .... m_ClipBox.Intersect( pObject2Device->TransformRect(rect).GetOuterRect()); .... } PVS-Studio warning: V1004 CWE-476 The 'pObject2Device' pointer was used unsafely after it was verified against nullptr. Check lines: 237, 248. cfx_psrenderer.cpp 248
The pObject2Device pointer may be null, as evidenced by checking this pointer for nullptr equality. However, further the pointer is dereferenced without prior verification.
V1004, the second example, is the SwiftShader library.
VertexProgram::VertexProgram(...., const VertexShader *shader) : VertexRoutine(state, shader), shader(shader), r(shader->dynamicallyIndexedTemporaries) { .... if(shader && shader->containsBreakInstruction()) { enableBreak = ....; } if(shader && shader->containsContinueInstruction()) { enableContinue = ....; } if(shader->isInstanceIdDeclared()) { instanceID = ....; } } PVS-Studio warning: V1004 CWE-476 The 'shader' pointer was used unsafely after it was verified against nullptr. Check lines: 43, 53. vertexprogram.cpp 53
The shader pointer can be null, as evidenced by checking this pointer for nullptr equality. However, further the pointer is dereferenced without prior verification.
Greetings to Google developers
The PVS-Studio team sends its greetings to Google’s developers and is ready to cooperate. It is possible to discuss at least two options:
- You can license the PVS-Studio product so that all the developers of Chrome, Chromium, as well as the authors of third-party libraries used in these projects can use it. Anyway, you can make all Google employees have a license.
- You can conclude a contract, within the framework of which our team will customize the PVS-Studio analyzer to the needs of the company, correct all errors that it can find, regularly conduct code audits and correct new errors.
Try PVS-Studio. Write to us . We will help with project verification and issue a temporary license for detailed information.
Conclusion
Thanks to everyone who got to the end of the series of articles. I hope you were interested.
As you can see, even in such a high-quality project as Chromium there are a lot of errors that the PVS-Studio analyzer is able to detect. I suggest you start using it in your projects.
If you want to share this article with an English-speaking audience, then please use the link to the translation: Andrey Karpov. Chromium: Miscellaneous Defects .
Source: https://habr.com/ru/post/348134/
All Articles