Hi Jakub, Any other comments? I'd like to finish this rather than leaving it in its current half-done state.
Wilco Hi, Jakub Jelinek wrote: On Fri, Dec 07, 2018 at 04:19:22PM +0000, Wilco Dijkstra wrote: >> The test case doesn't need an aligned object to fail, so why did you add it? > > It needed it on i686, because otherwise it happened to see the value it > wanted in the caller's stack frame. Right, so I fixed that by increasing the size of the frame in broken_setjmp to be larger than the frame in main, so it's now extremely unlikely to accidentally read from a random stack location and end up with a valid stack pointer. > >> + /* Compute expected next alloca offset - some targets don't align > >> properly > >> + and allocate too much. */ > >> + p = q + (q - p); > > > > This is UB, pointer difference is only defined within the same object. > > So, you can only do such subtraction in some integral type rather than as > > pointer subtraction. > > __builtin_setjmp is already undefined behaviour, and the stack corruption is > even more undefined - trying to avoid harmless theoretical undefined behaviour > wouldn't be helpful. > No, __builtin_setjmp is a GNU extension, not undefined behavior. Well the evidence is that it's undocumented, unspecified and causes undefined behaviour... > And > something that is UB and might be harmless today might be harmful tomorrow, > gcc optimizes heavily on the assumption that UB doesn't happen in the > program, so might optimize that subtraction to 0 or 42 or whatever else. > >> > And I'm not sure you have a guarantee that every zero sized alloca is at >> > the >> > same offset from the previous one. >> >> The above pointer adjustment handles the case where alloca overallocates. >> It passes on x86-64 which always adds 8 unnecessary bytes. > > What guarantee is there that it overallocates each time the same though? How could it not be? It could only vary if it was reading an uninitialized register or adding a random extra amount as a form of ASLR. But there is no point in trying to support future unknown features/bugs since it will give false negatives today. Wilco
