[compiler-rt] [llvm] scudo: default enabled dealloc_type_mismatch on new platforms (PR #146739)

[Christopher Ferris via llvm-commits]

cferris1000 wrote:

> > What is the motivation for doing this?
> > There are more than just android, trusty and fuchsia and having this strange check to set the flag feels like the wrong approach. Is there a reason this can't be a config change with the default off. Then you could add the config parameter as enabled in allocator_config.h to all of the configs.
> > This feels like something that's either on all of the time, or off all of the time, not enabled at run time.
> 
> The goal is to just have `dealloc_type_mismatch` be `true` by default.

But why is the goal to have it on by default? This is a change in behavior for everyone that isn't android/trusty/fuchsia. As I mentioned before, I know of at least another place this is used that is none of those, so you are going to change this and could cause aborts. And there might be others for which this could cause problems. We have made some behavioral changes in the past, but they don't result in a process aborting where it didn't before.

> However AFAIK it is off by default due to some bad software on Android. Just because it is off by default for Android, doesn't mean future platforms should make the same decision. Are you saying to move to a preprocessor macro that is defined by CMake, and then have downstream users change it?

No, I was talking about adding a config parameter to allocator_config.def. This would make the choice a compile time option instead of a runtime option. However, I was thinking this would probably not be the best idea since we might want to enable this and disable it at runtime on Android for specific code to allow us to enable this on some processes and not others. Other systems might want to do this too if they are running code not compiled by themselves.

Fundamentally, I don't believe forcing this change is good. If there was a way to indicate this should be on by default for new projects, that would be great. I don't think we have the ability yet. I would like to see this enabled for everyone, but changing the behavior now risks causing aborts that weren't there before.



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