[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

This revision was automatically updated to reflect the committed changes. Closed by commit rG840edd8ab262: [analyzer] Don't escape local static memregions on bind (authored by steakhal). Changed prior to commit: https://reviews.llvm.org/D139534?vs=484600&id=488546#toc Repository: rG LLVM Git

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. In D139534#4036783 , @steakhal wrote: > In D139534#4034719 , @xazax.hun > wrote: > >>> Here is the gis

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

steakhal added a comment. In D139534#4034719 , @xazax.hun wrote: >> Here is the gist of one *new* TP: > > Where would `sprops` get escaped? Did I miss that or was that reduced out of > the example? You are right, it 'never' escapes, yet in the past we m

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

xazax.hun added a comment. > Here is the gist of one *new* TP: Where would `sprops` get escaped? Did I miss that or was that reduced out of the example? Overall, this looks like a hard nut to crack. Escaping too much or too little are both problematic, and we don't have the information we need

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

steakhal added a comment. In D139534#4029097 , @xazax.hun wrote: > Sorry, I got a bit swamped, will try to take a look next week. In the > meantime, did you have a chance to run this over some open source projects? > Did you find any interesting diffs?

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

xazax.hun added a comment. Sorry, I got a bit swamped, will try to take a look next week. In the meantime, did you have a chance to run this over some open source projects? Did you find any interesting diffs? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

steakhal added a comment. Let me know if you have further concerns. @xazax.hun @NoQ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139534/new/ https://reviews.llvm.org/D139534 ___ cfe-commits mailing list

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

steakhal updated this revision to Diff 484600. steakhal added a comment. - Add two test cases demonstrating the false-positives this patch will introduce. These are semantically equivalent to the one mentioned in the comments by @NoQ. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

steakhal added a comment. Finally, I had some time to come back to this. Thanks for taking the time for such a detailed response, kudos! @NoQ In D139534#3999455 , @NoQ wrote: > Ok screw it, my static example is still broken. There's technically still a

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

NoQ added a comment. Ok screw it, my static example is still broken. There's technically still a leak in it because there's no time for the other code to kick in between `p = (int *)malloc(sizeof(int));` and `p = 0;` to read the value from `p`. But in practice there could be a lot of things goi

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

NoQ added inline comments. Comment at: clang/test/Analysis/malloc-static-storage.cpp:33-38 +void malloc_escape() { + static int *p; + p = (int *)malloc(sizeof(int)); + escape(p); // no-leak + p = 0; // no-leak +} NoQ wrote: > NoQ wrote: > > The main problem w

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

NoQ added inline comments. Comment at: clang/test/Analysis/malloc-static-storage.cpp:33-38 +void malloc_escape() { + static int *p; + p = (int *)malloc(sizeof(int)); + escape(p); // no-leak + p = 0; // no-leak +} NoQ wrote: > The main problem with static loca

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

NoQ added inline comments. Comment at: clang/test/Analysis/malloc-static-storage.cpp:33-38 +void malloc_escape() { + static int *p; + p = (int *)malloc(sizeof(int)); + escape(p); // no-leak + p = 0; // no-leak +} The main problem with static locals is that th

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

steakhal created this revision. steakhal added reviewers: NoQ, xazax.hun, Szelethus. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. Herald added a project: All. steakhal requested review of this revisio