Fri, Jun 14
@echristo Hi Eric, Could check this revision, please ? Is that patch Ok for you ?
would be to drop the size field to 0xFE, 0xFF, 0xFF, 0xFF.
Strange, I haven't got notification. Are Phab notifications working?
Anyway I think making Offset uint64_t instead of size_t will fix the issue.
I've not worked out why yet, it could be a 32/64-bit host issue.
Ironically bad-arm-attributes2.s test is failing on an Arm buildbot http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/4/steps/ninja%20check%201/logs/FAIL%3A%20lld%3A%3Abad-arm-attributes2.s
Thu, Jun 13
LGTM with one nit and Rui's comments addressed.
LGTM with the following changes.
Wed, Jun 12
Mon, Jun 10
Got it. I'll commit this patch on behalf of you.
I do not have access to anything. The only repo I currently have cloned is my fork of the official git repository.
Do you have a commit access?
I was not trying to force you to use lld in some particular way. I was just trying to help you by suggesting one solution that might work for some use cases, and I didn't know that that wouldn't work for you. If it doesn't work for you, you can continue using lld as a library as you are already doing. We support that as well, and a bug like this should be fixed.
That is not an acceptable solution. This compiler is an embedded, statically linked library that can be used by other programs. It is not a standalone compiler. If the library is statically linked, there is literally no way to start a new process without forcing every single user of my library to support their own program being called with a special option for the sole purpose of invoking a linker. Since one of the intended uses, for example, is a game engine script library, which would compile a webassembly script for a map, this compilation process cannot require the game to support calling it's own executable, or link the script engine as a DLL for the sole purpose of having the script engine then load it's own DLL in a new process. These are not reasonable restrictions to have on what is supposed to be a self-contained compiler.
Yes, I suspect that there are other globals that are not reset between multiple executions of this linker.
Fri, Jun 7
Wed, Jun 5
Tue, Jun 4
Maybe Reid was talking about this icon, was it there before?
Phabricator memes are the best thing about Phabricator :)
Wait, did someone enable phab memes in the last update? This is too silly. =p
@thakis : Updated the existing test to fail without this patch.
As trivial as it may seem, this is missing tests, even if it's for a few flags. Also please try to include context with your patches as it makes them much easier to review. If you have some sort of plan for actually bringing Mach-O LLD closer to ld64 feature-wise, I think llvm-dev may be a good place to actually outline your plan for this, which will give everyone a more clear picture of what future changes you have planned in mind.
Can you add a testcase for this? Also it's appreciated if the uploaded diffs are made with a larger context (diff -U999 or similar) to allow expanding the context here in phabricator.
@echristo What is your opinion on this patch ? Is it worth to integrate it ?
Mon, Jun 3
I'm fine with this patch. We've discussed a lot about what is a desirable value for a "null" for debug info, but looks like as long as it does not appear as a valid address, it should be fine.
Sun, Jun 2
ping. @ruiu Do you think it would be useful to integrate this patch ?
Tue, May 28
Remove unused var
Mon, May 27
deleted redundant tests.
Tue, May 21
I'm personally happy with the implementation now, but obviously there needs to be agreement from the other reviewers.
Mon, May 20
The commit description of rLLD361190 does not contain Differential Revision: https://reviews.llvm.org/D61711, so the revision was not closed when you committed that...