[all-commits] [llvm/llvm-project] c1e401: [lldb] Change the two remaining SInt64 settings in...

[Jason Molenda via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: c1e401f3624780f85f4c9a26960752ee3f37fafb
      https://github.com/llvm/llvm-project/commit/c1e401f3624780f85f4c9a26960752ee3f37fafb
  Author: Jason Molenda <jmolenda at apple.com>
  Date:   2024-08-22 (Thu, 22 Aug 2024)

  Changed paths:
    M lldb/source/Target/Target.cpp
    M lldb/source/Target/TargetProperties.td
    M lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
    A lldb/test/API/functionalities/memory/big-read/Makefile
    A lldb/test/API/functionalities/memory/big-read/TestMemoryReadMaximumSize.py
    A lldb/test/API/functionalities/memory/big-read/main.c

  Log Message:
  -----------
  [lldb] Change the two remaining SInt64 settings in Target to uint (#105460)

TargetProperties.td had a few settings listed as signed integral values,
but the Target.cpp methods reading those values were reading them as
unsigned. e.g. target.max-memory-read-size, some accesses of
target.max-children-count, still today, previously
target.max-string-summary-length.

After Jonas' change to use templates to read these values in
https://reviews.llvm.org/D149774, when the code tried to fetch these
values, we'd eventually end up calling OptionValue::GetAsUInt64 which
checks that the value is actually a UInt64 before returning it; finding
that it was an SInt64, it would drop the user setting and return the
default value. This manifested as a bug that target.max-memory-read-size
is never used for memory read.

target.max-children-count is less straightforward, where one read of
that setting was fetching it as an int64_t, the other as a uint64_t.

I suspect all of these settings were originally marked as SInt64 so a
user could do -1 for "infinite", getting it static_cast to a UINT64_MAX
value along the way. I can't find any documentation for this behavior,
but it seems like something Greg would have done. We've partially lost
that behavior already via
https://github.com/llvm/llvm-project/pull/72233 for
target.max-string-summary-length, and this further removes it.

We're still fetching UInt64's and returning them as uint32_t's but I'm
not overly pressed about someone setting a count/size limit over 4GB.

I added a simple API test for the memory read setting limit.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications