-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Give more info when a UB trap is detected in zig cc debug mode #5163
Comments
This is high on my wish list. Locally I've commented out |
I had started a pure-Zig port of miniubsan in #5165 |
I just wanted to share this, both for users looking for solutions to getting more information out of these UBSan traps and who come across this page as I did, as well as to make sure the devs are aware of this already existing alternative on the off-chance they don't know about it. I'm developing freestanding code with zig cc and wanted the benefits of getting more information than a trap allows. I've just discovered that passing I assume but have not tested that linking the ubsan library (like LakeByTheWoods does above) would allow non-freestanding users to get full functionality without implementing anything. In any case, it's good to know that it appears that full UBSan functionality is available to zig and zig cc users without having to patch the compiler. |
Hi - I'm keen to have a proper crack at this issue, in particular doing this comprehensively & solving for the different cases people might have. This includes supporting both the "full" & "minimal" ubsan functions. So first thing I want to establish, is that if we aren't just going to be using LLVM's ubsan handlers (see https://github.com/llvm-mirror/compiler-rt/blob/master/lib/ubsan), which benefits are we hoping to see in having custom zig-implemented versions of these functions? I've had a look at #5165 and the comments people had there - and will try and take those all into account, but basic rough ideas include:
I think ideally we would also provide options for making it easier for end-users to deal with ubsan traps (minimal or full-fat) with their own functions in some form if they want to as well - so that can better slot-into existing assertion/error-loggin or other debugging systems, whilst still providing sensible default behaviour. Should this be implemented incrementall (maybe addressing the minimal runtime first) with multiple PRs? Or should I look into just doing this as a full-fat feature with all bells & whistles up front. I was also lookig at using as a starting point #5165 if that was OK with @LemonBoy . Finally - this feels like it would be a fairly big feature - so are there any requirements for regression tests or similar to ensure this keeps working as intended? EDIT: Update 2021-07-32, nothing interesting to report, just that I am working on it bit by bit |
My intent is currently to create a PR that will have implementations for the full ubsan runtime & the minimal ubsan runtime. However - there's quite a lot that needs implementing for the full ubsan runtime - so I'm currently focusing on handling integer overflows specifically. If I were to create a PR (not ready for merge) sometime soon that just has that one specific case being handled. Would anyone please be able to give feedback on the overall approach? I think that would probably be better from my perspective to make sure that I'm generally heading in the right direction - as opposed to creating a full-on ubsan implementation that then does not really align with the goals of the zig project & therefore would not be merged or require significant refactoring to be merged. If so - I should be able to get something together for next weekend. |
In lieue of creating a full-on PR (because it is relatively minimal) - I've uploaded a gist containing an implementation of handling various integer overflows & division by 0 (alongisde minimal code copied from #5165 which I haven't really touched - I want to get "full" ubsan working first and then will try and come back and improve the minimal implementation accordingly). @LemonBoy some feedback on what I have so far would be great if you would please be able to provide some! Just because there are a decent number of handlers to implement so having feedback on the first few so I can apply that first before fleshing out further handlers would be great. https://gist.github.com/moosichu/ee3dd8407653640a36fb9625ac586082 |
Progress is still being made on this slowly but surely, feedback on the gist would still be great if anyone could take the time (or I'm happy to re-organise it into another format for those purposes). One of the questions I have is with regards to the following
So the panic system works quite well in showing where the error is occurring - however it slightly unfortunate that the error-handling code itself (the ubsan implementation) generates a couple of lines of noise (that maybe should be hidden?) before getting to the point of interest. I feel like you might ideally want to get error reports that look like this:
Is this something that would be possible to do? |
As a heads-up, one of the problems I'm running into is that some parts of ubsan seem to be only implementable in C++. A good example is the implementation of |
Becuase it looks simpler to implement and could get me a PR together that I can actually use as a precedent to understand what might be needed for the ubsan PR - would an implementation of ASAN traps be something that might be merged upstream for the C/C++ clang compiler? Similar to ubsan - you can provide the asan flag but the lack of exposed trapping procedures leads to a failed compile. |
Just realised I forgot to mentioned that I have picked this up again! See: https://github.com/moosichu/zigsan.git |
Just to NOTE: Luckily I decided to raise a bug when Oh, and the idea: "No Optimise" ==== "Debug mode" ... is not even close to reality. |
This should undo the effect of `-fsanitize-trap` that zig cc is passing internally. See ziglang/zig#5163 (comment)
This should undo the effect of `-fsanitize-trap` that zig cc is passing internally. See ziglang/zig#5163 (comment)
I tried to pass Since I am cross compiling to Linux arm64 / Darwin x86_64 + arm64, and I don't have that library at hand (for all systems), the only way to make it work was to pass BTW, I recompiled my app with clang and the component that was using UB turned out to be libunwind from llvm (llvm/llvm-project#91144) |
I'm sorry to jump out on that sort of issue again, because it was partially treated in issues such as #4830.
To comment @andrewrk in the previous issue :
The part about detecting issues in other code is 100% true. I've been using zig as a C compiler for some personal project and I've been detected already two UB in major code (nuklear and stb_printf).
But while this is incredibly useful at detecting faulty code, I think it could be better at giving out info about what's wrong. In the two cases, my workflow had been :
Just a plain crash without info is pretty harsh. I'm 100% in favor of not having UB code running, and I'm 100% in favor of detecting more UB code in the future, but I think right now we lack the tool to properly investigate the exact issue encountered by the UB sanitizer.
Maybe there's a flag that I'm missing. From clang doc, I tried
-fno-sanitize-recover=undefined
in my compile flags, but it did not worked.The text was updated successfully, but these errors were encountered: