delcypher wrote: @PiJoules
> This might be a bit orthogonal to this patch, but I think rather than using > the mangled dollar scheme used by `__builtin_verbose_trap` to encode the > UBSan description, it might be cleaner to instead use dwarf's > `DW_AT_description` to actually store the UBSan description. I think ideally > we'd have the artificial function you create just be the same as any of the > typical ubsan handler functions (like `__ubsan_handle_cfi_check_fail`) and > its arguments are the registers, memory locations, or values relevant to this > particular ubsan check. Then lldb could just print the function with its > arguments as well as a the `DW_AT_description` and from there the dev sees > what they need to inspect in lldb to see what's wrong. Since the function is > artificial, I think lldb or llvm-symbolizer should also be able to tell that > this isn't a real function and instead point to the original function where > the actual ubsan intrinsic was located. I wouldn't say it's orthogonal. It's a very interesting implementation idea but there are several reasons why we don't want to pursue it right now. 1. What you are proposing is significantly more complicated than what is implemented in this PR. This work being part of a GSoC project means it is time limited and we don't have time to fully implement what you are proposing. 2. The "managed dollar scheme" used by `__builtin_verbose_trap` is an already a well established scheme for encoding trap reasons that LLDB already understands. This approach is also used by `-fbounds-safety` (not upstreamed yet), and Swift. 3. An advantage of the current scheme in this PR is that the coupling between the debugger and compiler is very weak which means it is robust against mixing compiler and debugger versions (something that unfortunately happens in practice, despite my best efforts). The coupling is "weak" compared to what you are proposing because the DWARF consumer (typically a debugger like LLDB) just needs to parse the fake function name. The downside though is the "trap reason" is limited to being a static string and thus can only include information known at compile time. 4. Your proposed scheme has a much tigher coupling between the debugger and compiler because you're proposing different function names for each UBSan trap type and arguments to the fake functions (essentially a more complicated ABI). This is much less robust against mixing debugger and compiler versions. OTOH I think being able to refer dynamic values would be quite valuable. I think this PR is a substantial improvement over the status quo so I think we should land this (once we've addressed other reviewer comments) but we (the compiler) should reserve the right to change the implementation. https://github.com/llvm/llvm-project/pull/145967 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
