Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling of C++ new and delete #39

Open
devreal opened this issue Mar 16, 2016 · 4 comments
Open

Handling of C++ new and delete #39

devreal opened this issue Mar 16, 2016 · 4 comments

Comments

@devreal
Copy link

devreal commented Mar 16, 2016

I just tested your STACK tool with success as I found two bugs in our code base. Great work, many thanks! However, I came across a load of warnings that stem from C++ delete and delete[] operators, reported if the pointer to the deleted object was referenced before. The code:

int main()
{
   char *ptr = new char[128];
   *ptr = '\0';
   delete[] ptr;

   return 0;
}

produces the warning:


---
bug: anti-simplify
model: |
  %4 = icmp eq i8* %1, null, !dbg !15
  -->  false
stack:
  - /home/p/src/test/memory_test.cc:5:0
ncore: 1
core:
  - /home/p/src/test/memory_test.cc:4:0
    - null pointer dereference

This warning seems to be unnecessary since (I) ptr should never be NULL since new is supposed to throw an exception if the allocation fails and (II) delete should check for NULL values. Of course the situation is different for C malloc calls. Is there any reason why these warnings are included in your analysis of C++ codes? Is my understanding of the standard incorrect here?

@zeldovich
Copy link
Collaborator

I seem to recall that when Clang compiles a "delete" statement, it generates LLVM bytecode to check for whether the pointer being deleted is null (and if so, to not delete the object). Since STACK operates on the LLVM bytecode, it flags this "if" statement as dead code. Of course, the programmer didn't write this "if" statement, so we shouldn't be flagging it. But we couldn't find a convenient way of detecting whether this LLVM bytecode comparison was generated by Clang internally, or whether it corresponds to actual code written by the programmer (short of modifying Clang to emit special annotations for this delete check).

@xiw
Copy link
Owner

xiw commented Mar 17, 2016

@devreal glad it helped. may i ask which code base is it?

@zeldovich we tried several approaches and settled on using basic block names as a simple and effective heuristics. This is implemented in src/SimplifyDelete.cc.

However, in order to make it work well, you will need to build clang yourself. See the first entry of the FAQ file (or let us know if the latest clang doesn't emit blocks with such names).

@zeldovich
Copy link
Collaborator

Oh, that's right, the default build of Clang doesn't put in those basic block names.. Forgot all about that.

@devreal
Copy link
Author

devreal commented Mar 20, 2016

Thanks for your quick feedback and sorry for not checking the FAQ beforehand. I checked and do not see these false positives with the self-built versions. It is not clear to me which of the configure switches used in the INSTALL description actually triggers the addition of the block names, which are not there in the versions of LLVM and Clang that are shipped with my.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants