LGTM

LGTM

isn't the chance of this lower than a normal tag matching by accident (also producing an incorrect report)? The chance of this should be (16 / 256) * (1 / 256).

The reason it was done this way was that what looks like a short granule might not be one - sometimes a low tag value is simply a low tag value, and the last byte of the granule is user memory that can match by chance.

LGTM

LGTM

LGTM with a nit

The buildbot went green before your revert has landed.
No idea what's going on, but this change does not seem to be the cause of it.

Try reverting it in the meantime?

It appears that this change broke the sanitizer-windows bot:
https://lab.llvm.org/buildbot/#/builders/127/builds/12064

In D103845#2806211, @leonardchan wrote:

I think the newline gets added to the frame description only if Symbolizer::GetOrInit()->SymbolizePC(pc) is nonnull, so if it fails then no newline is added.

s/vfork/fork/ in the description

__asan_detect_use_after_return, any variant of it, is not entirely correct. First, using the presence of the global is better than its value, because the linker will pick a random instance in case they disagree, while &(...) != nullptr gives reliable OR semantics.

I think it is too hit-and-miss to add even under a flag. It's just not the right approach - but I also don't know what the right approach would be.

I'd be OK with removing it. 13 years should be enough time to deprecate features.

You are adding a new global to every translation unit.

• Private linkage would not allow them to be merged, this can have significant binary size and RAM overhead.
• There is no guarantee that any of these globals will end up to the left of any sanitized globals. With -fdata-sections, the linker is free to reorder.
• It is also not referenced from anything, so -gc-sections is likely to kill it.

LGTM

LGTM

LGTM

LGTM

LGTM

Added a comment in the test.

I'm not sure if this is worth the code savings.

Right, our commit process is a bit strange. Ideally, the change author should initiate commit after they have the necessary approvals, and then be responsible to deal with any fallout in a timely manner. As things are, the author does not control the timing of the commit, and the committer does not get the bot messages.

Just noticed D101385.
I'll revert this anyway to keep the bots green overnight.

The test is failing on https://lab.llvm.org/buildbot/#/builders/70/builds/6736/steps/7/logs/stdio

I fixed another one in https://reviews.llvm.org/rG561f4b9087457e9ea3cf4aeb57dcd507e2fa6258
Please watch the bots after you land changes like this one and don't let them stay broken for an entire day!

This is an alternative to D91617, it's not currently needed anywhere outside solaris/sparcv9. Let's shelve it for now.

I meant why __hwasan_tag_pointer is necessary, can't you match the address in the report header?
But I don't really care that much, LGTM if this is easier for any reason.

why is this necessary? there should be a memory address in the report header I think

LGTM

Nice.
If this is a significant optimization, we might want to request it through the allocator config and fail if it is impossible - in case we change the size class map in the future and break this by accident.

LGTM, thanks!

I don't think this is correct in general - check AArch64ExpandPseudoInsts.cpp for a long list of pseudos that are replaced with real machine instructions.

LGTM
I wonder if doing the size check before the DCZID check could speed up small allocations, and maybe raising the threshold value could help.
But we can worry about that later.

could you also update the test to check that the invalid access address from the report header matches one of the tag dump addresses?

I think this happens when the patch is uploaded through web ui and not through arc. It would be great if arc patch made it clear when that is the case!

split locks for live and free lists

addressed comments

LGTM

LGTM

LGTM

Add it to CXX_OPERATOR_ATTRIBUTE, on non-mac non-windows?

LGTM

I'm not working on this currently, but I would appreciate and support such change. It's a big diff, but IMHO it is the cleanest, most maintainable approach.

AArch64 had TBI from the start, but the kernel support was added only recently. Add untags only in the failing tests, for memory that is passed to system calls.

LGTM

LGTM

LGTM

LGTM

LGTM

Nice.
LGTM.

LGTM

Do you have commit access? I can land this for you.

LGTM

LGTM

Ha. Indeed, the tests run on the android bot.
LGTM

I'm concerned about losing test coverage of stack and globals in hwasan.
Could we keep both modes until page aliasing reaches feature parity?
As an alternative, we could setup aarch64 simulator testing, or even a native aarch64 builder/tester.