Fix T81340: UBSan: load of value .. not valid for GPU enum type
ClosedPublic
Actions

Authored by Ankit Meel (ankitm) on Oct 1 2020, 1:21 AM.

Details

Reviewers

Clément Foucault (fclem)

Maniphest Tasks

T81340: UBSan: several runtime errors at launch.

Commits

rBa31a87f8943a: T81340: UBSan: load of value .. not valid for GPU enum type

Summary

The underlying type of the enum cannot be fixed here due to its usage
in C code.

It seems that all the values possible in the underlying type are not
valid for an enum.
Only 0 to (2*max - 1) if all enumerators are unsigned.
Or -2*max + 1 to 2*max -1 if at least one value in the enum is
signed.

For the latter, we'll have to add a dummy enumerator with -1 value.
For the former, the macro asks for the biggest value among the
listed ones. If any enumerator C is set to say A|B, then C would
be the maximum.
(2*max-1) is used as the mask.

The warning:
GPU_vertex_buffer.h:43:1: runtime error: load of value 4294967291
which is not a valid value for type 'GPUVertBufStatus'

https://github.com/llvm/llvm-project/commit/1c2c9867

Ref T81340

Diff Detail

Repository: rB Blender

Event Timeline

Ankit Meel (ankitm) created this revision.Oct 1 2020, 1:21 AM

Harbormaster completed remote builds in B10501: Diff 29441.Oct 1 2020, 1:21 AM

Ankit Meel (ankitm) requested review of this revision.Oct 1 2020, 1:21 AM

Ankit Meel (ankitm) mentioned this in T81340: UBSan: several runtime errors at launch. .Oct 1 2020, 1:24 AM

It would be nice if we could avoid adding dummy enum types.

Couldn't we use the sizeof(enum)? or cast to the proper type before flipping the bits.

Missed adding a dummy to one of the enums.

Harbormaster completed remote builds in B10508: Diff 29454.Oct 1 2020, 8:30 AM

Ankit Meel (ankitm) planned changes to this revision.Oct 1 2020, 8:31 AM

sizeof(<enum>) is indeed a good idea. The problem with ~(0) is that:
If any enumerator is set to UINT_MAX, ~(0) will widen the underlying type to 8 bytes to accommodate the -1. This is like adding the *_MAX{/MIN} (but only when there's a limit used in the enum.)
If any enumerator is set to UINTMAX_MAX, ~(0) cannot be represented as any integer type and we get a [-Wenum-too-large].
https://en.cppreference.com/w/c/language/enum
https://godbolt.org/z/4qr5be Observe the sizes with and without the ~(0) in the enum Table.
Will change the mask to 1 << 8 * sizeof(_enum_type) in the macro, and remove the need of dummy DONT_USE-s.

Revert "Missed adding a dummy to one of the enums."
Revert "Fix T81340: UBSan: load of value .. not valid for GPU enum type"
Add maximum enum value to the macro.

It seems that all the values possible in the underlying type are not valid for an enum.
Only 0 to (2*max - 1) if all enumerators are unsigned. Or -2*max + 1 to 2*max -1 if at
least one value in the enum is signed. http://godbolt.org/z/6rTKEe

For the latter, we'll have to add a dummy enumerator with -1 value.
For the former, the macro asks for the biggest value among the listed ones. (2*max-1)
is used as the mask.

Harbormaster completed remote builds in B10527: Diff 29495.Oct 1 2020, 8:35 PM

Ankit Meel (ankitm) edited the summary of this revision. (Show Details)Oct 5 2020, 9:02 AM

Ankit Meel (ankitm) added a subscriber: Clément Foucault (fclem).

This looks like an acceptable workaround.

This revision is now accepted and ready to land.Oct 7 2020, 3:35 PM

Closed by commit rBa31a87f8943a: T81340: UBSan: load of value .. not valid for GPU enum type (authored by Ankit Meel (ankitm)). · Explain WhyOct 8 2020, 7:46 PM

This revision was automatically updated to reflect the committed changes.

Ankit Meel (ankitm) added a commit: rBa31a87f8943a: T81340: UBSan: load of value .. not valid for GPU enum type.