Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Undefined behavior adding 1 to nullptr #2661

Closed
no-more-secrets opened this issue Dec 17, 2021 · 4 comments
Closed

Undefined behavior adding 1 to nullptr #2661

no-more-secrets opened this issue Dec 17, 2021 · 4 comments

Comments

@no-more-secrets
Copy link
Contributor

no-more-secrets commented Dec 17, 2021

On master I am seeing some UB here:

../fmt/include/fmt/core.h:2485:13: runtime error: applying non-zero offset 1 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../fmt/include/fmt/core.h:2485:13 in

which is this line:

if (begin + 1 < end && begin[1] == '}' && is_ascii_letter(*begin) &&
      *begin != 'L') {

when begin is nullptr. But in that case it should be the case that end is nullptr as well, and so we can get around this by only adding 1 to begin when begin != end, which logically makes sense anyway, since if begin == end then begin + 1 < end will always be false anyway. That seems to prevent the begin + 1 from executing when begin == nullptr. I will send a PR for this.

@vitaut
Copy link
Contributor

vitaut commented Dec 17, 2021

Please provide a self-contained example (preferably in godbolt) that demonstrates the UB.

@no-more-secrets
Copy link
Contributor Author

@vitaut Here is a minimal reproducer: https://godbolt.org/z/KTYd95o1W

When I change dynamic_formatter<> to formatter<std::string> then the issue goes away, so it could be specific to dynamic_formatter.

@no-more-secrets
Copy link
Contributor Author

As an aside, I am using dynamic_formatter because in my real code the formatter in question is for a std::variant, and so I lifted the example from the comments in format.h. But now I am curious, why should we use dynamic_formatter in that situation as opposed to the regular formatter?

Also note that this is likely a recent regression because the code in question in my code base is pretty old, and I encountered this error in the process of upgrading from 7.3.1 to 8.0.1 today.

@vitaut
Copy link
Contributor

vitaut commented Dec 18, 2021

Fixed in 659de77, thanks for reporting.

why should we use dynamic_formatter in that situation as opposed to the regular formatter?

dynamic_formatter parses format specs at runtime when the information of an alternative stored in a variant is available.

@vitaut vitaut closed this as completed Dec 18, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants