-
Notifications
You must be signed in to change notification settings - Fork 257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clang generates wrong code on armeabi-v7a for string literal pointers (over-optimization) #642
Comments
Reduced test case: /*
/ssd/stand-arm-14-r16b/bin/clang -pie -fPIE literal.c -o literal -O2 && \
adb push literal /data/local/tmp && \
adb shell /data/local/tmp/literal
*/
int puts(const char *msg);
volatile int dummy = 0;
volatile int expected = 42;
volatile int failed = 0;
__attribute__((weak)) const char *identity(const char *val) {
return val;
}
__attribute__((weak)) void fail(const char *msg) {
puts(msg);
failed = 1;
}
#define X() do { dummy++; dummy++; dummy++; dummy++; dummy++; dummy++; } while (0)
int main(void) {
const char *const literal = "1";
const char *s = literal;
s = identity(s);
X(); X(); X(); X(); X(); X(); X(); X(); X(); X(); X();
X(); X(); X(); X(); X(); X(); X(); X(); X(); X(); X();
X(); X(); X(); X(); X(); X(); X(); X(); X(); X(); X();
if (expected != 42) {
fail("FAILURE-1");
}
if (s != literal) {
fail("FAILURE-2");
}
return failed;
} Output:
|
This issue has already been filed upstream as https://bugs.llvm.org/show_bug.cgi?id=32780. The upstream workaround was to turn off the constant pool promotion, which seems to be a size optimization: The compiler in the upcoming NDK r17 already leaves off the optimization off by default. To turn it off with the Clang in older NDKs (at least with r15c and up), add I reproduced this problem with r15c and r16b but not with r13b or r14b. The Clang in r13b/r14b also didn't recognize the |
@stephenhines You might be interested in this. I don't think there's anything more to do here, unless we want to try fixing the optimization upstream. It looks like someone tried fixing it about 9 months ago. (https://bugs.llvm.org/show_bug.cgi?id=32780#c12) I wonder how much of a size regression it's going to cause from r16 -> r17. The r17 compiler disables the constpool promotion optimization, which tries to embed small constant variables into |
I commented on the original bug and am following up with ARM about fixing/re-enabling that optimization. Thanks for tracking down the issue and finding the original bug. |
Thanks from me as well for finding the original bug. Just to follow up on the r14b aspect, I tried again and was not able to reproduce with r14b, so I probably just ran the wrong binary for that test. Sorry about that. |
Clang from NDK r16b targeting armeabi-v7a can over-optimize loads of string literal pointers such that the value of a
const char *
variable to which a string literal has been assigned changes depending on where the variable is referenced. Specifically, if the string is stored in a local constant block in the code stream and the second reference to the variable occurs past the point where that string can be referenced by an add/sub PC instruction, Clang may put a second copy of the string in a subsequent local constant block and load the variable's value by referencing that copy, resulting in a different pointer value than was originally stored in the variable.This is a regression from r13b (r14b and r15c are likewise broken; I haven't tested any other intermediate releases).
Test case: https://gist.github.com/achurch/3bca2069a0f25db98df764b6711dc83d
I haven't been able to minimize the test case as much as I'd like (in trivial code, the compiler seems to reload from the stack rather than loading a literal pointer with add/sub PC), but the bug occurs in
TEST_UTF8("\xF9\x80\x80\x80\x80", 0x1000000, 5)
because theif (_s != _string + _consume)
test in the macro loads the value of_string
from a different string literal than the initial assignment of_string
to_s
:Note that there is only one reference to the string literal within the macro; it assigns the string literal to a local variable precisely to avoid the possibility of two identical string literals having different addresses.
The text was updated successfully, but these errors were encountered: