On Fri, Feb 18 2022, Taylor Blau wrote:
> On Sat, Feb 19, 2022 at 03:48:42AM +0100, Ævar Arnfjörð Bjarmason wrote:
>> >> > +}
>> >> > +
>> >> > +# If [AL]SAN is in effect we want to abort so that we notice problems.
>> >> > +prepend_var XSAN_OPTIONS : abort_on_error=1
>> >>
>> >> XSAN_OPTIONS stands for "options that are common to all ?SAN", I
>> >> guess.
>> >
>> > I was also unclear on this. Looking around in the google/sanitizers
>> > repository, I don't see something called "XSAN_OPTIONS" mentioned
>> > anywhere (neither in documentation nor in the actual source code).
>> >
>> > Is this a convenience variable that we use to store options that are
>> > common to both ASAN_OPTIONS and LSAN_OPTIONS? If so, I am not sure the
>> > extra confusion is worth it, since it only contains abort_on_error=1.
>> >
>> > I guess it makes it (along with the prepend_var function introduced by
>> > this patch) possible for a caller to write XSAN_OPTIONS=... into their
>> > environment and then run a test script and have their settings feed into
>> > ASAN_OPTIONS and LSAN_OPTIONS. But I don't know that callers would find
>> > this super useful (or, at least not dramatically more convenient than
>> > setting both variables).
>> >
>> > I could be wrong, and I'm obviously biased towards my own usage of the
>> > ASAN/LSAN builds, but to me this patch might be clearer without the
>> > extra variable.
>>
>> Sorry, yes it's just a git.git invention to refer to "common LSAN and
>> ASAN' things. Perhaps GIT_ASAN_AND_ASAN_COMMON or something would be
>> much less confusing & better? Bikeshedding most welcome :)
>
> Yeah, I would be fine with something like GIT_ASAN_AND_LSAN_COMMON if it
> makes things easier. I think it definitely is clearer, and does make it
> easier to add new options to both.
>
> I probably wouldn't ever tweak its value myself, so I don't think I have
> a strong opinion here. If it were me, I'd probably just as soon
> duplicate setting `abort_on_error=1` in the ASan- and LSan-specific
> variables. But I don't have strong feelings here, so whatever makes the
> most sense to you is fine with me.
Once you get to tweaking some more advanced options it makes sense to
share a lot for both. One change I have locally on top of this is e.g.:
prepend_var XSAN_OPTIONS : dedup_token_length=9999
prepend_var XSAN_OPTIONS : log_exe_name=1
prepend_var XSAN_OPTIONS : log_path=\"$TEST_RESULTS_XSAN_FILE\"
I.e. for a feature to log machine-readable stacktraces when you run the
test suite with LSAN or ASAN.
prepend_var XSAN_OPTIONS : dedup_token_length=9999
prepend_var XSAN_OPTIONS : log_exe_name=1
prepend_var XSAN_OPTIONS : log_path=\"$TEST_RESULTS_XSAN_FILE\"
Which is how I ended up with XSAN_OPTIONS I think, i.e. I'd test things
with LSAN, switch to ASAN, would have to rename the variable
etc. Changing a single letter was easier ... :)
