Details

If we have a store to a piece of memory which is known constant, then we know the store must be storing back the same value. As a result, the store (or memset, or memmove) must either be down a dead path, or a noop. In either case, it is valid to simply remove the store.

The motivating case for this involves a memmove to a buffer which is constant down a path which is dynamically dead.

Note that I'm choosing to implement the less aggressive of two possible semantics here. We could simply say that the store *is undefined*, and prune the path. That seemed a bit too likely to break real code in subtle ways, so I decide to just remove the store itself.

Why do you think a store through constant would break subtly? For most users I'd expect a store to a constant to correspond with a write to a read-only page causing a loud crash.

I assume you mean before this patch right? If so, then yes and no. It's possible the constant location is in a readonly page, but I have a lot of downstream constant locations which are intermixed with writeable data. I'm sure other frontends might have the same. Your reasoning may hold specifically for global constants though.

Why do you think a store through constant would break subtly? For most users I'd expect a store to a constant to correspond with a write to a read-only page causing a loud crash.

I assume you mean before this patch right? If so, then yes and no. It's possible the constant location is in a readonly page, but I have a lot of downstream constant locations which are intermixed with writeable data. I'm sure other frontends might have the same. Your reasoning may hold specifically for global constants though.

I was just surprised at the claim that this might be a common unnoticed bug. Putting constants in read only memory is a technique decades older than UBSan and I regard anything the latest UBSan can catch is something that we're safe to optimize on.

I don't think this patch is wrong per se, I'm just quite confident it's safe to make it stronger. If you don't agree, you can leave the stronger optimization for another author in the future. Either way, separate patches would make it easier to find the cause of a regression.

hi reames, I have a question in the view of customer. If he writes deliberately to a read-only memory to expect trigger a seg fault, or if he wants to write to a memory which he does not realize it is read only. Should compiler tell him the writing is invalid, rather than treat it as a nop silently?

hi reames, I have a question in the view of customer. If he writes deliberately to a read-only memory to expect trigger a seg fault, or if he wants to write to a memory which he does not realize it is read only. Should compiler tell him the writing is invalid, rather than treat it as a nop silently?

See the discussion in the commit thread about this being undefined behaviour and possibilities for sanitization.