8000 Fix some UBSAN false positives by kevinbackhouse · Pull Request #6115 · ruby/ruby · GitHub
[go: up one dir, main page]
Skip to content

Fix some UBSAN false positives #6115

New issue 8000

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

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 12, 2022
Merged

Fix some UBSAN false positives #6115

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a13f81a
Fix some UBSAN false positives.
kevinbackhouse Jul 11, 2022
e05286a
ruby tool/update-deps --fix
kevinbackhouse Jul 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fix some UBSAN false positives.
  • Loading branch information
@kevinbackhouse
kevinbackhouse committed Jul 11, 2022
commit a13f81a9a9782555e448c3b062866e0263c3c00b
2 changes: 1 addition & 1 deletion include/ruby/internal/arithmetic/long.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ RB_INT2FIX(long i)
/* :NOTE: VALUE can be wider than long. As j being unsigned, 2j+1 is fully
* defined. Also it can be compiled into a single LEA instruction. */
const unsigned long j = i;
const unsigned long k = 2 * j + RUBY_FIXNUM_FLAG;
const unsigned long k = (j << 1) + RUBY_FIXNUM_FLAG;
Copy link
Member
@jhawthorn jhawthorn Jul 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(We discussed this a little at work)

It seems like, even though C considers unsigned overflow defined for both << and *, UBSAN checks for overflow on *, but not <<. We figured that this UBSAN behaviour is probably safe to rely on, and doing this avoids having to NO_SANITIZE in this code that is exported in the C extension API.

(The output of gcc and clang w/o UBSAN is identical)

👍

const long l = k;
const SIGNED_VALUE m = l; /* Sign extend */
const VALUE n = m;
Expand Down
6 changes: 3 additions & 3 deletions parse.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,12 +232,12 @@ enum {
};

#define NUMPARAM_ID_P(id) numparam_id_p(id)
#define NUMPARAM_ID_TO_IDX(id) (unsigned int)(((id) >> ID_SCOPE_SHIFT) - tNUMPARAM_1 + 1)
#define NUMPARAM_IDX_TO_ID(idx) TOKEN2LOCALID((tNUMPARAM_1 + (idx) - 1))
#define NUMPARAM_ID_TO_IDX(id) (unsigned int)(((id) >> ID_SCOPE_SHIFT) - (tNUMPARAM_1 - 1))
#define NUMPARAM_IDX_TO_ID(idx) TOKEN2LOCALID((tNUMPARAM_1 - 1 + (idx)))
static int
numparam_id_p(ID id)
{
if (!is_local_id(id)) return 0;
if (!is_local_id(id) || id < (tNUMPARAM_1 << ID_SCOPE_SHIFT)) return 0;
unsigned int idx = NUMPARAM_ID_TO_IDX(id);
return idx > 0 && idx <= NUMPARAM_MAX;
}
Expand Down
3 changes: 3 additions & 0 deletions regparse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

#include "regparse.h"
#include <stdarg.h>
#include "internal/sanitizers.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of this addition we need to run ruby tool/update-deps --fix (this is the cause of CI failing). It's a little finicky, ping me if you'd prefer I run it to fix the deps 😅.


#define WARN_BUFSIZE 256

Expand Down Expand Up @@ -394,6 +395,8 @@ str_end_cmp(st_data_t xp, st_data_t yp)
return 0;
}

NO_SANITIZE("unsigned-integer-overflow", static st_index_t str_end_hash(st_data_t xp));

static st_index_t
str_end_hash(st_data_t xp)
{
Expand Down
0