A few questions:
1) I'm not clear precisely what problem this patch fixes. It's true
that some people have incorrectly assumed that basic asm clobbers memory
and this change would fix their code. But some people also incorrectly
assume it clobbers registers. I assume that's why Jeff Law proposed
making basic asm "an opaque blob that read/write/clobber any register or
memory location." Do we have enough problem reports from users to know
which is the real solution here?
Whenever I do something for gcc I do it actually for myself, in my own
best interest. And this is no exception.
Seems fair. You are the one putting the time in to change it.
But do you have actual code that is affected by this? You can't really
be planning to wait until v7 is released to have your projects work
correctly?
The way I see it, is this: in simple cases a basic asm behaves as if
it would clobber memory, because of the way Jeff implemented the
asm handling in sched-deps.c some 20 years ago.
Look for ASM_INPUT where we have this comment:
"Traditional and volatile asm instructions must be considered to use
and clobber all hard registers, all pseudo-registers and all of
memory."
The assumption that it is OK to clobber memory in a basic asm will only
break if the asm statement is inlined in a loop, and that may happen
unexpectedly, when gcc rolls out new optimizations.
That's why I consider this to be security relevant.
I'm not sure I follow. Do you fear that gcc could mistakenly move the
asm into a nearby loop during optimization (resulting in who-knows-what
results)? Or is there some way that any basic asm in a loop could have
some kind of a problem?
But OTOH you see immediately that all general registers are in use
by gcc, unless you declare a variable like
register int eax __asm__("rax");
then it is perfectly OK to use rax in a basic asm of course.
According to the docs, that is only supported for global registers. The
docs for local register variables explicitly say that it can't be used
as input/outputs for basic asm.
And if we want to have implicitly clobbered registers, like the
diab compiler handles the basic asm, then this patch will
make it possible to add a target hook that clobbers additional
registers for basic asm.
I think we should try to avoid changing the semantics in v7 for memory
and then changing them again in v8 for registers.
IOW, if I see some basic asm in a project (or on stack overflow/blog as
a code fragment), should I assume it was intended for v6 semantics? v7?
v8? People often copy this stuff without understanding what it does.
The more often the semantics change, the harder it is to use correctly
and maintain.
2) The -Wbasic-asm warning patch wasn't approved for v6. If we are
going to change this behavior now, is it time?
Yes. We have stage1 for gcc-7 development, I can't think of a better
time for it.
I would even say, the -Wbasic-asm warning patch makes more sense now,
because we could warn, that the basich asm clobbers memory, which it
did not previously.
After your patch has been checked in, I'll re-submit this.
4) There are more basic asm docs that need to change: "It also does not
know about side effects of the assembler code, such as modifications to
memory or registers. Unlike some compilers, GCC assumes that no changes
to either memory or registers occur. This assumption may change in a
future release."
Yes, I should change that sentence too.
Maybe this way:
"Unlike some compilers, GCC assumes that no changes to registers
occur. This assumption may change in a future release."
Is it worth describing the fact that the semantics have changed here?
Something like "Before v7, gcc assumed no changes were made to memory."
I guess users can "figure it out" by reading the v6 docs and comparing
it to v7. But if the semantic change has introduced a problem that
someone is trying to debug, this could help them track it down.
Also, I'm kind of hoping that v7 is the 'future release.' If we are
going to change the clobbers, I'd hope that we'd do it all at one time,
rather than spreading it out across several releases.
If no one is prepared to step up and implement (or at least defend) the
idea of clobbering registers, I'd like to see the "This assumption may
change in a future release" part removed. Since (theoretically)
anything can change at any time, the only reason this text made sense
was because a change was imminent. If that's no longer true, it's time
for it to go.
dw
