Thanks for the write up. I agree that it might be a good change. Would be interesting to scan the existing packages to see how many do rely on binary prefix replacement (maybe even looking in a few larger environments could be useful, to get an idea).

But yes, I also had to debug some issues with std::string(PREFIX) in the past and it's definitely tricky.

Would be interesting to scan the existing packages to see how many do rely on binary prefix replacement (maybe even looking in a few larger environments could be useful, to get an idea).

I agree. It could also be helpful to make it a loud warning for a couple of releases and see if anybody reports anything interesting.

But yes, I also had to debug some issues with std::string(PREFIX) in the past and it's definitely tricky.

tl;dr I don't think that this issue is specific to std::string, except for the fact that I don't think modern compilers are as smart at optimizing C code yet, so I would not want to chalk this up to C strings really being safe either.

Longer answer:

I should be clear here. The previous instances of binary prefix replacement causing problems with strings that I've seen -- this one also from RAPIDS and the original conda-build issue, both of which I found out about after rapidsai/cudf#18251 -- both boil down to std::string not being null-terminated resulting in differences in how strings are printed. That is absolutely a pain to debug, but in the grand scheme of things it is debuggable: at least you are looking at localized code and when debugging the normal control flow of the program you can use normal C++ debugging techniques in gdb with a debug build to see that string's length does not match its contents. Also, those issues are strictly due to differences in how std::string and C strings are represented, so once you get to that point knowing how the language works is sufficient to understand the problem and trace it back to the prefix patching.

rapidsai/cudf#18251 is far worse. The exact same optimization that I observed is, so far as I know, completely valid for a C string; it is only related to std::string insofar as I suspect that modern C++ compilers have more advanced heuristics to optimize code C++ strings than C strings, and because the fact that c strings are null terminated means that the probability of finding one string that is a substring of another is far lower because it also requires that they end the same way, i.e. it cannot be an arbitrary substring in the middle. Here, the compiler decided that it would be a good idea for one part of the code to reuse a substring of the string constant embedded by a completely different part of the code. The error was not localized in any way, in that the prefix string being replaced was from an error in a completely different part of the code than where string operations were performed. In fact, based on the fact that the header where the prefix was embedded is included in every single file in our library, since I only see the prefix string for that error once in the final binary I think the two constants may actually have come from separate TUs and been combined at link-time. Furthermore, since the string constant was just passed as a literal to a std::string function and this probably was only observed when compiled with -O2 or higher, there was no way to see anything wrong by inspecting the contents of a std::string in a debug session for inconsistencies between the content and the data. The only way to really root cause the issue was to disassemble near the point of error, inspect a section of assembly instructions manually, identify all possible instructions referencing memory in specific regions, find one that referenced a null string character, then attempt to remap the mapped memory regions from the process space (via info proc mappings or equivalent) back to the original sections of the ELF file (as determined via readelf and hexdump or equivalents).

Hence why I think we should change the default here: these kinds of errors are almost impossible to debug without deep knowledge of the conda/rattler build/install stack, the C/C++ languages, and the actual ELF binary format, and they can manifest in arbitrarily bad ways, not just misprinted strings.