[libcxx-commits] [libcxx] [libc++][string] Replace ASAN volatile wrapper with memory barrier (PR #184693)

[Vitaly Buka via libcxx-commits]

vitalybuka wrote:

> > > Having an opt-in attribute lik e `[[as an_sbo_container]]` or something should be perfectly fine, no? Then any code for that class would have the performance penalty, but that's already the case with volatile/memory barriers.
> > 
> > 
> > Oh, I see. My "idea" of proper solution was: "no additional changes in user code". To my taste it's better than barrier as we don't need to think about placement of barriers, but not significantly better? I guess this is a new option to consider. Thanks!
> 
> The improvement I see is that the compiler will do the correct thing, and the library doesn't have to add workarounds anymore. "If you annotate parts of an object, apply this attribute to it" seems rather trivial to understand. I'd also expect this would actually improve performance, since the compiler knows what we're trying to tell it and don't just add a memory barrier for _everything_. I haven't thought about this deeply, so I don't know whether I'd missing something problematic here.

I agree, adding memory barriers are terrible, but here another case

We had to revert  https://github.com/llvm/llvm-project/pull/193649 cause by same issue.
Looks like std::variant<std::string...>. Probably another barrier may help. 
Unfortunately now barrier needs to be in std::variant :(

And I guess [[as an_sbo_container]] will not help for variant as well.

Some imaginary example:

```
// in a.cpp
void create_string(char* buff) {
  new(buff)std::string();
}

// in b.cpp
void test(bool string){
  char buff[1000];
  if (string)
     create_string(buff);
  else
     create_something_else(buff);
  // do stuiff;
}
```
I don't see how in example above we can make compiler to avoid speculative loads from buff in "do stuff" part.

@AdvenamTacet Would you like to take a look and reproduce?
cc @boomanaiden154 

So I suspect short string annotation is flawed, maybe it has to be either make optional, or even disabled competently.
Now it blocks us from having string annotations, because of false positives like this.

As workaround maybe we could always allocate buffer if we enable string annotations. Not sure how this will affect abi. @philnik777



https://github.com/llvm/llvm-project/pull/184693