https://github.com/clayborg commented:
LGTM
https://github.com/llvm/llvm-project/pull/71828
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/walter-erquinigo closed
https://github.com/llvm/llvm-project/pull/71828
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/felipepiovezan approved this pull request.
LGTM. I'm ok postponing the lifetime change to a separate task
https://github.com/llvm/llvm-project/pull/71828
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
return std::make_unique(
walter-erquinigo wrote:
Sure, I've just changed it to assert
https://github.com/llvm/llvm-project/pull/71828
___
lldb-commits
https://github.com/walter-erquinigo updated
https://github.com/llvm/llvm-project/pull/71828
>From b7fadbbe01d626fbffe2523c601b2e7ea727b591 Mon Sep 17 00:00:00 2001
From: walter erquinigo
Date: Thu, 9 Nov 2023 11:40:23 -0500
Subject: [PATCH] [LLDB] Ensure the data of apple accelerator tables is
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
return std::make_unique(
JDevlieghere wrote:
This checks internal consistency so an `assert` is totally appropriate imho.
https://github.com/llvm/llvm-project/pull/71828
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
return std::make_unique(
walter-erquinigo wrote:
@JDevlieghere , what do you suggest in this case? Plain assert or better not to
put anything? I'd rather follow the guidelines
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
return std::make_unique(
JDevlieghere wrote:
Without expressing an opinion, the [contributing
guidelines](https://lldb.llvm.org/resources/contributing.html) say not to add
new `lldbasserts`. This
https://github.com/walter-erquinigo updated
https://github.com/llvm/llvm-project/pull/71828
>From 48ba23b4c0ef9c698b26d06e5b8b337a774563b8 Mon Sep 17 00:00:00 2001
From: walter erquinigo
Date: Thu, 9 Nov 2023 11:40:23 -0500
Subject: [PATCH] [LLDB] Ensure the data of apple accelerator tables is
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
return std::make_unique(
clayborg wrote:
> These asserts might not always pass? I don't know if it's guaranteed that if
> one of those sections is present, then the rest of them are as well.
You
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
return std::make_unique(
clayborg wrote:
Fine with me!
https://github.com/llvm/llvm-project/pull/71828
___
lldb-commits mailing list
https://github.com/felipepiovezan edited
https://github.com/llvm/llvm-project/pull/71828
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
return std::make_unique(
felipepiovezan wrote:
@clayborg what do you think about moving ownership of the data to inside the
AppleAcceleratorTable class?
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
return std::make_unique(
walter-erquinigo wrote:
These asserts might not always pass? I don't know if it's guaranteed that if
one of those sections is present, then the rest of them are as well.
https://github.com/clayborg commented:
See inline comment
https://github.com/llvm/llvm-project/pull/71828
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create(
return std::make_unique(
clayborg wrote:
DataExtractor objects don't always have shared buffers. I am guessing in this
case it does have them, but you might throw in a `lldbassert(...)` just in
https://github.com/clayborg edited
https://github.com/llvm/llvm-project/pull/71828
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
walter-erquinigo wrote:
The changes seem very trivial to me, so please let me know if there's something
else I should do or be aware of.
https://github.com/llvm/llvm-project/pull/71828
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
felipepiovezan wrote:
> The correct fix seems to also store the underlying storage along with the
> accelerator tables in AppleDWARFIndex.
This was my initial reaction as well. Is there some blocker to doing this?
https://github.com/llvm/llvm-project/pull/71828
https://github.com/bulbazord commented:
The idea of this makes sense to me, but I think you should let @felipepiovezan
take a look first.
https://github.com/llvm/llvm-project/pull/71828
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Walter Erquinigo (walter-erquinigo)
Changes
lldb's AppleDWARFIndex is created with a few `llvm::AppleAcceleratorTable`
variables, but they only hold pointers to the underlying data, which doesn't
prevent the data owner (lldb's
https://github.com/walter-erquinigo created
https://github.com/llvm/llvm-project/pull/71828
lldb's AppleDWARFIndex is created with a few `llvm::AppleAcceleratorTable`
variables, but they only hold pointers to the underlying data, which doesn't
prevent the data owner (lldb's DataBufferHeap)
22 matches
Mail list logo
