[PATCH] D39551: [analyzer] Make __builtin_debugtrap() a sink

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun abandoned this revision.
xazax.hun added a comment.



In https://reviews.llvm.org/D39551#914360, @dcoughlin wrote:

> I believe that the intent of `__builtin_debugtrap()` is to provide an in-source mechanism so that the developer can trap into the debugger and then continue execution. For example, in the Swift codebase this is used in combination with a debug flag to break into the debugger at the beginning a specified pass. https://github.com/apple/swift/blob/master/lib/SILOptimizer/PassManager/PassManager.cpp#L339


Oh, ok. I think this use is very convincing.

> Since the intent is to continue after the trap, I don't think the analyzer should treat it as a sink.

I see. I misinterpreted slightly the intent. From the discussion I linked:

> This patch is to fix radar://8426430. It is about llvm support of
>  `__builtin_debugtrap()`
>  which is supposed to consistently raise SIGTRAP across all systems. In
>  contrast,
>  `__builtin_trap()` behave differently on different systems. e.g. it raises
>  SIGTRAP on ARM, and
>  SIGILL on X86. The purpose of `__builtin_debugtrap()` is to consistently
>  provide "trap"
>  functionality, in the mean time preserve the compatibility with on gcc on
>  `__builtin_trap()`.

So according to my interpretation `__builtin_debugtrap()` is not only a way to express that the user would like to continue with a debugger but also to provide consistent behavior across platforms. Also, I assumed this is mainly used for cases where something already went wrong, so it might make sense to suppress results on that path which might be the side effect of error that triggered to take that path in the first place. But the PassManager example you provided proves this wrong.

> Did you get requests from users to treat it like a sink? Or false positives that would be resolved if were a sink? Or is this something you noticed by inspection?

I was just surprised by this behavior when I saw this happening and wanted to make sure whether this is intended. I did not get any user request.


https://reviews.llvm.org/D39551