Skip to content
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

read_only define not working as intended with Clang #369

Open
wmpmiles opened this issue Dec 8, 2024 · 4 comments
Open

read_only define not working as intended with Clang #369

wmpmiles opened this issue Dec 8, 2024 · 4 comments

Comments

@wmpmiles
Copy link

wmpmiles commented Dec 8, 2024

The define here:

# define read_only __attribute__((section(".rodata")))

Does not work as intended. It currently poisons the .rodata section and makes it both readable and writable. I tested this with Clang 18.1.3 both by running a simple test of writing to a read_only variable without segfaulting and by checking the section attributes with objdump -h.

The behavior between GCC and Clang seem to be the same in terms of the generated assembly. They both insert a .section section_name, "aw", @progbits directive before the memory initialization. It's just that GCC warns you that this is modifying the .rodata section attributes while Clang issues no such error.

Unfortunately, I don't believe there's any way to define a variable as part of a read-only section with Clang, as __attribute__((section("section_name"))) will always give the section "aw" attributes, which will always make the section in the final ELF image writable.

There's a hack-y workaround for this with GCC, described in this StackOverflow answer. To get this to work with modern GCC you do:

#define read_only __attribute__((section("section_name, \"a\", @progbits #")

Which uses the correct attributes and comment delimiter. This works in GCC because it doesn't sanitize the section name, so we can inject our own section attributes and then comment out the "aw", @progbits that it usually inserts. Clang, however, does sanitize the section name, so there's no way to comment out the unwanted section attributes.

I can think of two ways to fix this:

  1. Continue to use the __attribute__((section("section_name"))) attribute and add a post-compilation script or program that reads the ELF header and patches the section to read-only.
  2. Define all of your read-only variables using inline assembly or in a separate assembly file, which gives you proper control over your section attributes.

There may be a better way that I've missed. Only fiddled with this for a little while.

@mmozeiko
Copy link
Contributor

mmozeiko commented Dec 8, 2024

There is simpler alternative that does not rely on compiler custom attributes or inline assembly tricks. const keyword:

#define read_only const

const globals will be placed in read-only section automatically. So:

read_only int x[] = { 1, 2, 3};

...
x[0] = 3;   // does not compile
int* y = x; // will produce warning, but that can be disabled because raddbg does not care about "constness"
y[0] = 3;   // will crash as expected

@wmpmiles
Copy link
Author

wmpmiles commented Dec 8, 2024

I had assumed that read_only existed and was being used within the code base because of the semantic differences between it and const.

const doesn't guarantee that a global variable will be placed into a read-only section at a language standards level. Some compilers may make this guarantee.

There are also certain constructs where one may want something to be read-only without being const.

If the usage of read_only aligns with const then that would be an easy fix, yes.

@mmozeiko
Copy link
Contributor

mmozeiko commented Dec 8, 2024

Right, it does not guarantee it. But all compilers raddbg targets does support that by default.

My assumption is that read_only was like that only because originally code was C++, and in C++ int* y = x; would not compile. So you would not be able to pass those read only globals to arbitrary functions or assigning to some struct members expecting non-const pointers/arrays without explicit casts. But now in C it does work fine.

@wmpmiles
Copy link
Author

wmpmiles commented Dec 8, 2024

Thought I had tested int* y = &some_const_x, but was misinterpreting the compiler error. Yeah, sounds like const should work fine if you can guarantee that const globals are placed into .rodata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants