vsk added a comment. In https://reviews.llvm.org/D34299#788379, @filcab wrote:
> In https://reviews.llvm.org/D34299#788152, @vsk wrote: > > > The source locations aren't constants. The ubsan runtime uses a bit inside > > of source location structures as a flag. When an issue is diagnosed at a > > particular source location, that bit is atomically set. This is how ubsan > > implements issue deduplication. > > > It's still an `llvm::Constant`. Just like in StaticData, in line 2966. > Basically, I don't see why we need to add the store/load and an additional > indirection, since the pointer is constant, and we can just emit the static > data as before. My earlier response made the assumption that you wanted a copy of the source location passed to the runtime handler, by-value. I see now that you're wondering why the source locations aren't part of the static data structure. That's because the location of the return statement isn't known at compile time, i.e it's not static data. The location depends on which return statement is executed. > We're already doing `Data->Loc.acquire();` for the current version (and all > the other checks). The other checks do not allow the source location within a static data object to change. >>> I'd also like to have some asserts and explicit resets to nullptr after use >>> on the ReturnLocation variable, if possible. >> >> Resetting Address fields in CodeGenFunction doesn't appear to be an >> established practice. Could you explain what this would be in aid of? > > It would be a sanity check and help with code reading/keeping in mind the > lifetime of the information. I'm ok with that happening only on `!NDEBUG` > builds. > > Reading the code, I don't know if a `ReturnLocation` might end up being used > for more than one checks. If it's supposed to be a different one per check, > etc. > If it's only supposed to be used once, I'd rather set it to `nullptr` right > after use (at least when `!NDEBUG`), and `assert(!ReturnLocation)` before > setting. It's not a big deal, but would help with making sense of the flow of > information when debugging. Sure, I'll reset it to Address::invalid(). https://reviews.llvm.org/D34299 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
