This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH][gdb] fix unsigned overflow in charset.c
> On Oct 9, 2018, at 1:57 PM, John Baldwin <jhb@FreeBSD.org> wrote:
>
> On 10/9/18 10:40 AM, Paul Koning wrote:
>>
>>
>>> On Oct 9, 2018, at 1:31 PM, Pedro Alves <palves@redhat.com> wrote:
>>>
>>> On 10/09/2018 06:19 PM, Paul Koning wrote:
>>>> This fixed an overflow in pointer arithmetic that crashes GDB on Mac OS.
>>>
>>> _unsigned_ overflow? That isn't undefined. Do we really want to trap
>>> those? I don't think GCC's version does that.
>>>
>>> From:
>>> https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#silencing-unsigned-integer-overflow
>>> seems like there's a way to disable it.
>>>
>>> Thanks,
>>> Pedro Alves
>>
>> You're right, it was an LLVM build. I know unsigned overflow is well defined with integers; is that true for pointers?
>>
>> Given that GDB triggers this issue, should the GDB build do that LLVM workaround if LLVM is used to build it?
>>
>> But it seems simpler to use the proposed patch; clearly the intent is to back up a pointer by -(space_left) and doing that operation on a signed type seems like a logical thing to do, it makes the intended meaning clear.
>
> I also ran into the same failure using LLVM's ubsan on FreeBSD but in a different
> use of obstack_blank_fast(). If we wanted to fix this, I wonder if we'd instead
> want to fix it centrally in obstack_blank_fast (e.g. by using a ptrdiff_t cast)
> rather than fixing various consumers of the API. That would be a change to
> libiberty though, not just gdb.
I suppose. But casts in macros scare me, they can hide mistakes. It seems more reasonable to have the caller be responsible for creating a value of the correct type. Since it's an adjustment, I suppose the cast should be for ptrdiff_t rather than ssize_t?
paul