code segment access for CONSTRUCT_ON_FIRST_USE in c++20

[JuniorHsu]

abseil/abseil-cpp#1661 suggests CONSTRUCT_ON_FIRST_USE could trigger restricted memory access.
It looks scary to me given CONSTRUCT_ON_FIRST_USE is used all over the envoy.

[wbpcode]

The CONSTRUCT_ON_FIRST_USE is a good way to create a singleton and avoid possible Static Initialization Order Fiasco.
It may cannot work well in some cases, better tests are best way to prevent that.

cc @jmarantz

[jmarantz]

IIUC the problem here is that CONSTRUCT_ON_FIRST_USE winds up finding a different ctor than the one intended. I don't understand why the issue description suggests 'restricted memory use'.

The same problem can exist without that macro -- the change in C++ library winds up making the code do the wrong thing; not much to do about that other than hope that tests cover it.

I imagine it's not a very common use of CONSTRUCT_ON_FIRST_USE; but I haven't checked. A workaround would be to change the using to be a simple struct with a constructor that delegates to the string_view construction pattern you want.

[JuniorHsu]

Restricted memory might not be precise but the map could read code segment. If we replace string view with string, it triggers OOM since it tries to allocate 2^64 - 1 bits

[jmarantz]

That sounds good. That will leave no ambiguity about the wrong behavior due to the wrong constructor being called, but it has nothing to do with "restricted memory".

[JuniorHsu]

Sure I modified the title to make it more precise.

As for Fiasco, does constexpr could do for most of the cases? I have a hunch that we don't use constexpr since envoy doesn't start with c++17. CONSTRUCT_ON_FIRST_USE has one more benefits of less memory usage once that variable is never retrieved tho.

[JuniorHsu]

A scenario to hit:
one filter constructs a flat_hash_map by CONSTRUCT_ON_FIRST_USE from the proto, which could lead this bug and trigger unexpected behavior. It's tricky to assume the unit test on that filter cover this case.

[jmarantz]

So what you are saying is that if we have code that is not covered by unit tests, it might fail?

[JuniorHsu]

Yeah it might fail if we don't cover the corner cases. Code coverage tool can't catch that.

[jmarantz]

just to be clear, this has nothing to do with CONSTRUCT_ON_FIRST_USE. And it has nothing to do with flat_hash_map. It has to do with ambiguity in the constructor for string_view, and if you get that wrong you crash. Is that right?

So I don't think this is a subtle corner case issue.

[JuniorHsu]

I tried to reproduce without CONSTRUCT_ON_FIRST_USE with a plain operator new but I fail to reproduce. abseil folks suggests CONSTRUCT_ON_FIRST_USE could be one factor but I'm not able to tell.