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

Fix LTO linker warning #380

Closed

Conversation

frankbielig
Copy link

warning: ‘__builtin_strlen’ reading 1 or more bytes from a region of size 0 [-Wstringop-overread]

`warning: ‘__builtin_strlen’ reading 1 or more bytes from a region of
size 0 [-Wstringop-overread]`

Signed-off-by: Frank Bielig <[email protected]>
@joto
Copy link
Member

joto commented Nov 4, 2024

Can you explain why this gets rid of the warning? It seems to me the reason for the warning is something else and this "fix" only confuses the compiler enough that it doesn't print the warning any more. Problem is that this doesn't really save us, there are other cases where this warning shows up. So we still have to disable that warning.

There shouldn't be cases where set_user() is called with a nullptr.

@frankbielig
Copy link
Author

frankbielig commented Nov 4, 2024

It is really the only place I get this warning. It is triggered by

::osmium::memory::Buffer buffer(10000);
::osmium::builder::RelationBuilder relationBuilder(buffer);

relationBuilder.set_user(relation.user());`

The variable relation is the parameter of a ::osmium::handler::Handler::relation() callback. Do I have to check the user against nullptr at this point?

@joto
Copy link
Member

joto commented Nov 5, 2024

It is really the only place I get this warning.

That might be true in your code, but other code has similar problems in different places.

relationBuilder.set_user(relation.user());

Unless you are doing something strange relation.user() will never return a nullptr! So your check should never be necessary. The reason for the warning is different, it has something to do with the way we allocate memory and instantiate OSM objects in our buffer and then add additional information like the user name after the instantiated objects.

I believe your change merely seems to make the code sufficiently more complex that the compiler can't be sure any more and doesn't produce the warning. If you compile the libosmium tests you will see a different place showing the same problem where your fix doesn't work. We need a better way to fix this problem. :-(

@frankbielig
Copy link
Author

Ok. Then I will withdraw my PR.

@frankbielig frankbielig closed this Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants