ApolloAuto
Fixing potential issues
Hi there! We've embarked on a thorough examination of the Apollo project, utilizing static analysis tools to build a detailed history of code issues. This deep dive into the Apollo has revealed several key issues. Notably, we found that similar issues had been identified and resolved in other parts of the project, which highlights the problematic code's similarities and potential dangers. These lingering issues, we believe, significantly increase the risk of system crashes.
For reference to similar correction patterns, please see commit 42fb17ee in the file modules/dreamview/backend/sim_control_manager/sim_control_manager.cc.
@DoughIt Thank you for your suggestion. I want to know the details. Are we just using the scanning tool, or do we understand the code? I found that some places are not so reasonable.
For example, you can explain the problems based on examples, or classify the problems
Thank you for your response! We did not attempt to understand the complete business code but explored some historical fixed cases, thereby inferring whether there are noteworthy issues among the residual.
First, let's illustrate with a specific example. In commit 215f9888, we noticed that in the file modules/dreamview/backend/sim_control_manager/sim_control_manager.cc, at line 56, a new condition was added to check whether model_ptr_ is a non-null pointer. If it's not null, the Stop() method would be called. However, the logic here seems to have been inverted. The condition actually returns true when model_ptr_ is a nullpointer, and attempting to execute model_ptr_->Stop() in this case leads to a null pointer dereference issue.
This issue was later fixed in commit 42fb17ee (see the screenshot below).
From these fixed patterns, we infer that similar issues in the code are likely to cause problems in the future and then get fixed. For example, in this pr, as shown in the screenshot below, at line 98, even after confirming that model_ptr_ is null, the code still attempts to execute the model_ptr_->Stop() method, which could trigger a null pointer exception.
