davide added a comment.
I think I understand why this is failing:
AssertionError: False is not True : 'expression ((char**)environ)[0]'
returns expected result, got '(char *) $0 = 0x7ffeefbff753
"COMMAND_MODE=unix2003"'
Config=x86_64-/Users/davide/work/llvm-monorepo/build-release/bin/clang-7.
davide added a comment.
Yes, this needs a test. Thanks!
https://reviews.llvm.org/D44693
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide accepted this revision.
davide added a comment.
LGTM, thanks. Do you have commit access or you need somebody to commit this on
your behalf?
https://reviews.llvm.org/D44693
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://list
davide added a comment.
Allright, we got this one.
davide@Davidinos-Mac-Pro ~/w/l/l/lldb> git llvm push
Pushing 1 commit:
55f24c19d1c [Core] Correctly handle float division in Scalar.
Sendinglldb/trunk/source/Core/Scalar.cpp
Sendinglldb/trunk/unittests/Core/ScalarTest.cpp
Tran
This revision was automatically updated to reflect the committed changes.
Closed by commit rL328649: [Core] Correctly handle float division in Scalar.
(authored by davide, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D44693
Files:
lld
This revision was automatically updated to reflect the committed changes.
Closed by commit rL328658: Use the DWARF linkage name when importing C++
methods. (authored by davide, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D40283?vs=1
davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.
This needs a testcase.
Repository:
rL LLVM
https://reviews.llvm.org/D44998
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
davide added a comment.
I'm not particularly worried about testing double-free behavior, FWIW.
Ideally we should, but, I really understand it's a PITA. I think we might get
this for free when testing msan/asan (or just running under valgrind), assuming
there was already coverage for this path.
(
davide added a comment.
@tromey thanks! do you need somebody to commit this for you?
https://reviews.llvm.org/D44907
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added a comment.
Just remove it I'd say (bonus point if you can remove other bits)
https://reviews.llvm.org/D44752
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added a comment.
OK, I'll commit it for you later today.
https://reviews.llvm.org/D44907
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added a comment.
`lldb-test` for this purpose will be great. there should be examples in `lit/`.
Repository:
rL LLVM
https://reviews.llvm.org/D44998
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
davide added a comment.
I think this is almost ready to go in modulo minors. I'll let also @labath
comment on it. Thanks for your contribution!
Do you need somebody to commit this on your behalf?
Comment at:
packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/Test
davide accepted this revision.
davide added a comment.
LGTM. I'll commit for you once Greg reviews it again.
Comment at:
packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c:1
+__attribute__((section("__codesection")))
+int f(int a) {
kbaladu
davide abandoned this revision.
davide added a comment.
I think this is obsolete by now.
https://reviews.llvm.org/D42656
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide closed this revision.
davide added a comment.
Lang committed this a while ago (r323163)
Repository:
rL LLVM
https://reviews.llvm.org/D41997
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
This revision was automatically updated to reflect the committed changes.
Closed by commit rL328985: [Core] Grab-bag of improvements for Scalar.
(authored by davide, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D44907?vs=139844&id=14
davide added a comment.
Thanks Greg. This is a very large patch, but it seems mostly churn. I'll try to
find the time to review it carefully tomorrow.
https://reviews.llvm.org/D45170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://
davide added a comment.
Can you add another test or two? It's a little complicated to see what's going
on here, but from your description, it makes sense.
I'm not particularly worried right now to distinguish between `UNSUPPORTED` and
`PASS`. In practice, it doesn't matter (at least for the tran
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
Sorry for getting in here late. This seems to be a great improvement on the
state of the art, and given it's only enabled for the CMake build, I see little
harm not going forward with it.
In p
davide accepted this revision.
davide added a comment.
LGTM
https://reviews.llvm.org/D45518
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide created this revision.
davide added reviewers: jingham, friss, JDevlieghere, aprantl, labath, clayborg.
This allows us to collect useful metrics about lldb debugging sessions.
I thought that an example would be better than a thousand words:
Process 19705 stopped
* thread #1, queue = '
davide added a comment.
In https://reviews.llvm.org/D45547#1065054, @jasonmolenda wrote:
> Ah, no you couldn't set up a command alias like that. Still, if the full
> name was statistics, you could type 'stat' and it would match. 'stats'
> wouldn't, though.
>
> I do think decoupling the disabl
davide added a comment.
I prefer having it as a top level command rather than part of log. If you think
about it LLVM does the same distinction and it worked quite well in practice.
We plan to collect more metrics to the command so I'd very much like to have
this living as a separate entity.
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
No objections from me.
https://reviews.llvm.org/D45480
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
davide added a comment.
lgtm
https://reviews.llvm.org/D45573
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added a comment.
I'm under the impression that we should either merge `log timers` with `stats`
or just remove log timers altogether and start from scratch.
From what I hear from Jim, it was really useful for a few people, so maybe a
fresh start would be a better way of handling things. T
davide added inline comments.
Comment at: lldb/source/Commands/CommandObjectStats.cpp:43-46
+target->RegisterStats(StatisticKind::ExpressionSuccessful);
+target->RegisterStats(StatisticKind::ExpressionFailure);
+target->RegisterStats(StatisticKind::FrameVarSuccess);
+
davide added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c:14-15
+// This line adds a real body to the function, so we can set more than one
+// breakpoint in it.
+printf("Observable side effect\n");
davide added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c:14-15
+// This line adds a real body to the function, so we can set more than one
+// breakpoint in it.
+printf("Observable side effect\n");
davide updated this revision to Diff 142301.
https://reviews.llvm.org/D45547
Files:
lldb/include/lldb/Target/Target.h
lldb/include/lldb/lldb-private-enumerations.h
lldb/packages/Python/lldbsuite/test/functionalities/stats/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/stats/
davide added a comment.
In https://reviews.llvm.org/D45547#1066348, @jingham wrote:
> Timers seemed like they would be really useful for collection of data about
> operations in lldb, but for most things I think they end up being hard to use
> because actual wall-clock time is so variable from
davide added inline comments.
Comment at: lldb/source/Commands/CommandObjectStats.cpp:37
+
+if (target->GetCollectingStats() == true) {
+ result.AppendError("stats already enabled");
xiaobai wrote:
> nit: You can drop the `== true`
Thanks, I'll fix these
davide added inline comments.
Comment at: lldb/source/Commands/CommandObjectExpression.cpp:626
+ Target *target = m_interpreter.GetExecutionContext().GetTargetPtr();
+ if (!target)
jingham wrote:
> This is what CommandObject::GetSelectedOrDummy target is for.
davide added a comment.
Thanks for this! Just one minor inline.
Comment at: source/Breakpoint/BreakpointResolverFileLine.cpp:132
+if (is_relative) {
+ // If the path was reltive, make sure any matches match as long as the
+ // relative parts of the path match the
davide added inline comments.
Comment at: source/Breakpoint/BreakpointResolverFileLine.cpp:219-221
+ if (is_relative) {
+search_file_spec.GetDirectory().Clear();
+ }
For consistency with the rest of the codestyle (and what you use above), can
you remove th
davide added subscribers: clayborg, jingham, jasonmolenda, labath.
davide added a comment.
thanks!
Repository:
rL LLVM
https://reviews.llvm.org/D45592
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman
davide added subscribers: jasonmolenda, davide.
davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.
This requires an unittest (or an lldb-test test). Some comments inline.
I think @clayborg or @jasonmolenda are in a better position to r
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330043: [Command] Implement `statistics` command. (authored
by davide, committed by ).
Herald added a subscriber: llvm-com
davide added a comment.
In https://reviews.llvm.org/D45547#1066044, @xiaobai wrote:
> I really like this idea! It will be very helpful for @sas and I. I'd like to
> +1 creating a separate `stats dump` subcommand instead of dumping stats on
> `stats disable`.
Thanks! If you has ideas or want t
davide added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1773-1781
+static SectionType getSectionType(llvm::StringRef section_name) {
+ llvm::StringRef mapped_name;
+ if (section_name.startswith(".zdebug")) {
+mapped_name = section_name.drop
davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.
Some minor comments, almost ready to go. Do you have commit access or you need
somebody to push this on your behalf?
Comment at:
packages/Python/lldbsuite/test/li
davide added a comment.
This is fine, I'll commit it for you today.
https://reviews.llvm.org/D45628
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide accepted this revision.
davide added a comment.
lg
https://reviews.llvm.org/D46395
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide accepted this revision.
davide added a comment.
Thanks for taking the time to split this!
https://reviews.llvm.org/D46529
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide created this revision.
For reference/discussion.
GCC complains about signed-vs-unsigned comparison. I'm actually surprised that
`m_registers_count` is a `signed` integer, as I can hardly imagine a negative
register count. I'm under the impression that we could change
`m_register_count` f
davide created this revision.
https://reviews.llvm.org/D32148
Files:
include/lldb/Utility/StringLexer.h
source/Utility/StringLexer.cpp
Index: source/Utility/StringLexer.cpp
===
--- source/Utility/StringLexer.cpp
+++ source/Util
davide added a comment.
Thanks!
https://reviews.llvm.org/D32148
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL300729: [Utility/StringLexer] Remove dead code. (authored by
davide).
Changed prior to commit:
https://reviews.llvm.org/D32148?vs=95519&id=95788#toc
Repository:
rL LLVM
https://reviews.llvm.org/D321
davide added a comment.
I'm getting more convinced the warning emitted by GCC is bogus
/home/davide/work/llvm-lldb/tools/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp:58:26:
warning: comparison between signed and unsigned integer expressions
[-Wsign-compare]
assert
davide added a comment.
I reduced it:
https://gist.github.com/dcci/01c10b405041fa8d139a4f71aec102f7
https://reviews.llvm.org/D32137
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added a subscriber: nlewycky.
davide added a comment.
And I was wrong. @nlewycky explained on IRC:
14:23 < nlewycky> gcc is correct, in 'char + char' each char gets promoted to
'int' first then summed, then you've got an unsigned int == int comparison
14:23 < nlewycky> uint8_t and uns
davide added a comment.
Reference for the future
(http://www.open-std.org/jtc1/sc22/open/n2356/conv.html [conv.prom1])
https://reviews.llvm.org/D32137
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l
davide added a comment.
In https://reviews.llvm.org/D32137#731727, @labath wrote:
> Thanks for digging into this, I've learned something new today. Fixing this
> with a cast seems reasonable then.
Me too, apparently C++ integer promotion is less obvious than I thought ;)
https://reviews.llvm
This revision was automatically updated to reflect the committed changes.
Closed by commit rL300845: [Utility] Placate another GCC warning. (authored by
davide).
Changed prior to commit:
https://reviews.llvm.org/D32137?vs=95485&id=95961#toc
Repository:
rL LLVM
https://reviews.llvm.org/D3213
davide created this revision.
The goal of this patch is twofold:
First, it removes a wrong comment (at least, not correctly describing what the
function does).
Then, it rewrites the function to use a stringswitch where the registers are
enumerated explicitly instead of being computed programmati
davide added inline comments.
Comment at: source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1916
+ std::string Name = std::string(reg_info->name);
+ bool IsCalleeSaved = llvm::StringSwitch(Name)
+.Cases("r12", "r13", "r14", "r15",
Currently this uses two ca
This revision was automatically updated to reflect the committed changes.
Closed by commit rL312501: [ABI] Rewrite RegisterIsCalleeSaved. (authored by
davide).
Changed prior to commit:
https://reviews.llvm.org/D37420?vs=113675&id=113790#toc
Repository:
rL LLVM
https://reviews.llvm.org/D3742
davide created this revision.
If you pass an invalid compiler/debugger path on the cmdline to `dotest.py`
this is what you get.
$ python dotest.py --executable /home/davide/work/build-lldb/bin/lldb
--compiler /home/davide/work/build-lldb/bin/clandasfaasdfsg
Traceback (most recent call last
davide added inline comments.
Comment at: packages/Python/lldbsuite/test/dotest.py:56
+sys.exit(-1)
return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
lemo wrote:
> minor: can we print quotes around fpath? (this usually helps in messages to
davide updated this revision to Diff 119917.
https://reviews.llvm.org/D39199
Files:
packages/Python/lldbsuite/test/dotest.py
Index: packages/Python/lldbsuite/test/dotest.py
===
--- packages/Python/lldbsuite/test/dotest.py
+++ pac
davide added a comment.
Apologies, I generally do (but I forgot this time), let me update the patch.
Repository:
rL LLVM
https://reviews.llvm.org/D39199
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
davide added a comment.
If there are cases where `is_exe` shouldn't be fatal, then we might consider
splitting the utility into two bits (exists & is_exe).
In my mind, if you're checking whether something is executable you should run
the check on a valid entity, and it's up to the caller guarant
davide added inline comments.
Comment at: packages/Python/lldbsuite/test/dotest.py:52
def is_exe(fpath):
+"""Returns true if fpath is an executable.
clayborg wrote:
> We could add a default parameter here like:
>
> ```
> def is_exe(fpath, fatal=False):
>
davide added a comment.
In https://reviews.llvm.org/D39199#904169, @zturner wrote:
> I just wanted to make sure nobody was *already* relying on that behavior. If
> we can get away with being strict, we should be strict.
I agree. From a quick skim I don't see anything really relying on that, b
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316393: [lldbtests] Handle errors instead of crashing.
(authored by davide).
Changed prior to commit:
https://reviews.llvm.org/D39199?vs=119917&id=119965#toc
Repository:
rL LLVM
https://reviews.llvm
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
I was going to propose the same :)
Maybe an heads up on lldb-dev?
Thanks!
https://reviews.llvm.org/D39215
___
lldb-commits mailing list
lldb-com
davide added a comment.
I think we should just use the `_COMPILER` variant and default to the clang in
tree (wheter possible)
https://reviews.llvm.org/D39215
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
davide added a comment.
Thanks, I'll try this patch tomorrow.
I know this may be a little off, but how hard is to write a test for this so
that it doesn't regress?
https://reviews.llvm.org/D39307
___
lldb-commits mailing list
lldb-commits@lists.llv
davide added a comment.
Thanks for taking care of the test.
About the libmath question. I thought libmath exported a symbol named `a` ,but
I haven't checked the `readelf` output.
This is what I saw for the symbols:
(lldb) image lookup --address 0x777ec290
Address: libm.so.6[0x00
davide added a comment.
Sorry, my bad, I haven't grepped properly, there are 3 internal symbols named
`a` in `libm.so`.
https://reviews.llvm.org/D39307
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
davide added a subscriber: rsmith.
davide added a comment.
Richard Smith (@rsmith) owns clang and is familiar with this.
https://reviews.llvm.org/D39307
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
davide added a comment.
can you please try adding a test for this one? :)
https://reviews.llvm.org/D39314
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added a comment.
In https://reviews.llvm.org/D39314#908115, @sas wrote:
> In https://reviews.llvm.org/D39314#908065, @davide wrote:
>
> > can you please try adding a test for this one? :)
>
>
> To be fully honest, I'm not 100% sure how I'm supposed to add tests for this.
> I looked throug
davide requested changes to this revision.
davide added a comment.
This change should be unit-testable, no?
https://reviews.llvm.org/D39315
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb
davide added a comment.
In https://reviews.llvm.org/D39315#908246, @sas wrote:
> Same thing here, I have no idea how to go about testing something specific
> like this. Given that this is Windows Phone, it's even worse than simply
> Windows Desktop because the only way to test is by doing remot
davide created this revision.
davide added reviewers: aprantl, JDevlieghere, friss, zturner.
https://reviews.llvm.org/D56511
Files:
lldb/scripts/Python/python-swigsafecast.swig
lldb/scripts/Python/python-wrapper.swig
Index: lldb/scripts/Python/python-wrapper.swig
===
davide updated this revision to Diff 180924.
davide added a comment.
Address comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56511/new/
https://reviews.llvm.org/D56511
Files:
lldb/scripts/Python/python-swigsafecast.swig
lldb/scripts/Python/python-wrapper.swig
Index: lldb/s
This revision was automatically updated to reflect the committed changes.
Closed by commit rL350769: [Python] Update PyString_FromString() to work for
python 2 and 3. (authored by davide, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/
davide created this revision.
davide added reviewers: JDevlieghere, friss, zturner, aprantl.
Herald added a reviewer: serge-sans-paille.
In python 2, strings and bytes are the same, but they're not in
python 3, hence the return of read() needs an explicit conversion.
While I'm around, rename the r
davide added a comment.
Given the name is really bizarre, I would stick with what I have now, which at
least clarifies the intent (unless you feel strongly about it)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56517/new/
https://reviews.llvm.org/D56517
This revision was automatically updated to reflect the committed changes.
Closed by commit rL350788: [Python] Update checkDsymForUUIDIsOn to be
compatible with Python 3. (authored by davide, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.o
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
I hit an incarnation of this problem, and I was able to get away with a
localized fix in:
[lldb] r344982 - [ValueObject] Stop assuming types are non-zero sized.
Of course, your approach is pr
davide created this revision.
davide added a reviewer: clayborg.
rdar://problem/46886288
https://reviews.llvm.org/D57213
Files:
lldb/include/lldb/Utility/Scalar.h
lldb/source/Utility/RegisterValue.cpp
lldb/source/Utility/Scalar.cpp
lldb/unittests/Utility/ScalarTest.cpp
Index: lldb/unit
davide added a comment.
@clayborg, this is fairly mechanical, but I would appreciate if you can sign it
off.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57213/new/
https://reviews.llvm.org/D57213
___
lldb-commits mailing list
lldb-commit
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57222/new/
https://reviews.llvm.org/D57222
___
lldb-commit
davide marked 2 inline comments as done.
davide added inline comments.
Comment at: lldb/source/Utility/RegisterValue.cpp:159
case 32:
+case 64:
if (buffer.length % sizeof(uint64_t) == 0) {
aprantl wrote:
> I'm curious, everything else in this patc
davide added a comment.
Some comments. Thanks!
Comment at: include/lldb/Utility/FileCollector.h:5-7
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
You might want to update the license.
=
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
I don't have other comments, this is good to go for me. Pavel?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54617/new/
https://reviews.llvm.org/D54617
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
LGTM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57272/new/
https://reviews.llvm.org/D57272
___
lldb-commits mailing list
lldb-commits
davide added a comment.
Thanks for the review, Greg!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57213/new/
https://reviews.llvm.org/D57213
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/l
davide marked an inline comment as done.
davide added inline comments.
Comment at: lldb/source/Utility/Scalar.cpp:161
+if (endian::InlHostByteOrder() == eByteOrderBig) {
+ swapped_words[0] = apint_words[7];
+ swapped_words[1] = apint_words[6];
zturn
davide added inline comments.
Comment at:
packages/Python/lldbsuite/test/expression_command/radar_47565290/TestClassTemplateSpecializationParametersHandling.py:5-6
+
+from __future__ import print_function
+
+
I think this is not needed.
Commen
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
LGTM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55376/new/
https://reviews.llvm.org/D55376
___
lldb-commits mailing list
lldb-commits
davide added a comment.
the changes that touch code I wrote lgtm.
Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:789-792
+ assert(encodedBits.repr.unused == 0);
+ decodedBits.repr.sign = encodedBits.repr.sign;
+ decodedBits.repr.fraction = encodedBits.repr.fraction;
davide added inline comments.
Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:789-792
+ assert(encodedBits.repr.unused == 0);
+ decodedBits.repr.sign = encodedBits.repr.sign;
+ decodedBits.repr.fraction = encodedBits.repr.fraction;
+ decodedBits.repr.exponent = decode
davide added a comment.
To clarify, I think we ought to fix the UB regardless, but Zachary's change can
go in anyway as it doesn't make the situation worse than it was before.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57413/new/
https://reviews.llvm.org/D57413
__
davide marked an inline comment as done.
davide added inline comments.
Comment at: lldb/source/Utility/Scalar.cpp:168
+ swapped_words[6] = apint_words[1];
+ swapped_words[7] = apint_words[0];
+ apint_words = swapped_words;
davide wrote:
> aprantl w
davide marked an inline comment as done.
davide added inline comments.
Comment at: lldb/source/Utility/Scalar.cpp:168
+ swapped_words[6] = apint_words[1];
+ swapped_words[7] = apint_words[0];
+ apint_words = swapped_words;
zturner wrote:
> davide w
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
LGTM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57467/new/
https://reviews.llvm.org/D57467
___
lldb-commits mailing list
lldb-commits
201 - 300 of 573 matches
Mail list logo
