xazax.hun created this revision. xazax.hun added reviewers: NoQ, dcoughlin, Szelethus, baloghadamsoftware, haowei. xazax.hun added a project: clang. Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet.
I was running the FuchsiaHandleCheck over some code and tried to collect and distill some false positives due to symbol escaping not working properly for integers. After writing down all those test cases I feel like it is not always trivial what to do, so I decided to upload and annotate them so we can discuss here in depth what is the right way to deal with them. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D71041 Files: clang/test/Analysis/fuchsia_handle.cpp
Index: clang/test/Analysis/fuchsia_handle.cpp =================================================================== --- clang/test/Analysis/fuchsia_handle.cpp +++ clang/test/Analysis/fuchsia_handle.cpp @@ -28,6 +28,7 @@ void escape1(zx_handle_t *in); void escape2(zx_handle_t in); +void (*escape3)(zx_handle_t) = escape2; void use1(const zx_handle_t *in ZX_HANDLE_USE); void use2(zx_handle_t in ZX_HANDLE_USE); @@ -114,14 +115,6 @@ zx_handle_close(sb); } -void checkNoLeak07(int tag) { - zx_handle_t sa, sb; - if (zx_channel_create(0, &sa, &sb) < 0) - return; - escape1(&sa); - escape2(sb); -} - void checkLeak01(int tag) { zx_handle_t sa, sb; if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated here}} @@ -204,3 +197,109 @@ sc = t + sa; zx_handle_close(sc); } + +// Various escaping scenarios + +zx_handle_t *get_handle_address(); + +void escape_last_pointer_die01() { + zx_handle_t sb; + // When the result of get_handle_address dies its pointee should be escaped, + // as it might be modified elsewhere. + // Because of arrays, structs, the suggestion is to escape when whe no longer + // have any pointer to that symbolic region. + if (zx_channel_create(0, get_handle_address(), &sb)) + return; + zx_handle_close(sb); +} + +struct object { + zx_handle_t *get_handle_address(); +}; + +void escape_last_pointer_die02(object &o) { + zx_handle_t sb; + // Same as above. + if (zx_channel_create(0, o.get_handle_address(), &sb)) + return; + zx_handle_close(sb); +} + +void escape_last_pointer_die03(object o) { + zx_handle_t sb; + // Should we consider the pointee of get_handle_address escaped? + // Maybe we only should it consider escaped if o escapes? + if (zx_channel_create(0, o.get_handle_address(), &sb)) + return; + zx_handle_close(sb); +} + +void escape_through_call(int tag) { + zx_handle_t sa, sb; + if (zx_channel_create(0, &sa, &sb) < 0) + return; + escape1(&sa); + if (tag) + escape2(sb); + else + escape3(sb); +} + +struct have_handle { + zx_handle_t h; + zx_handle_t *hp; +}; + +void escape_through_store01(have_handle *handle) { + zx_handle_t sa; + if (zx_channel_create(0, &sa, handle->hp) < 0) + return; + handle->h = sa; +} + +have_handle global; +void escape_through_store02() { + zx_handle_t sa; + if (zx_channel_create(0, &sa, global.hp) < 0) + return; + global.h = sa; +} + +have_handle escape_through_store03() { + zx_handle_t sa, sb; + if (zx_channel_create(0, &sa, &sb)) + return {0, nullptr}; + zx_handle_close(sb); + return {sa, nullptr}; +} + +void escape_structs(have_handle *); +void escape_transitively01() { + zx_handle_t sa, sb; + if (zx_channel_create(0, &sa, &sb)) + return; + have_handle hs[2]; + hs[1] = {sa, &sb}; + escape_structs(hs); +} + +void escape_top_level_pointees(zx_handle_t *h) { + zx_handle_t h2; + if (zx_channel_create(0, h, &h2)) + return; + zx_handle_close(h2); +} // *h should be escaped here. Right? + +void quasi_escape() { + zx_handle_t sa, sb; + if (zx_channel_create(0, &sa, &sb)) + return; + if (sa == 5) { + // Here we no longer have a symbol. Is that right? + // So we can no longer refer to the state in the GDM. + // So the value in some sense escaped our analysis. + zx_handle_close(sa); + } else + zx_handle_close(sa); + zx_handle_close(sb); +}
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits