Updated according to int3's guide. Yeah this way is much more simplified indeed...

I removed ForceLoad enum and add LoadFrom & LoadScope. LoadFrom indicates currently where is this archive from and LoadScope means what symbols should we load from this archive (only lazy load, load ObjC symbols, load all symbols or just skip loading this time). Now addFile accepts a LoadFrom enum so it is able to know if this archive is from CLI or LC_LINKER_OPTION. Based on this LoadFrom enum and the LoadFrom enum in its cache (if any), we can decide what symbols we need to load this time. I think this could be clearer than our previous ForceLoad? Also, I added a table in Config.h's comment to explain my implementation when encountered with these flags, please check it for me to make sure that I understand this right!

I just realized that -force_load should not work only if the same library is already explicitly passed from command line, but not when it is previously passed from LC_LINKER_OPTION. I revised how I determine if -force_load should work by saving libraries shown in command line in a DenseSet.

In D129062#3636389, @int3 wrote:

lgtm, thanks! Do you have commit permissions or do you need me to land this for you?

Update according to int3's advice.

Pass reloc by reference instead of by value. Remove the unnecessary assertion. Use toString(Symbol *) instead of getName().

Remove some unnecessary code. Add comment for handleDtraceReloc function. Replace IR tests with assembly tests.

In D126464#3628819, @thakis wrote:

What --author flag should I use for you when landing this?

ping

Hi, sorry to interrupt but I just forgot to mention that I didn't have access to commit. Anyone can please help me land this one if it looks ok?

ping

Sorry! I'm a novice at LLVM and I just didn't realize that those types can be implicitly cast to ArrayRef ... I have changed those and it should be fine now!

More refactoring following Johannes's suggestion

Add tests by introducing llvm-ar to produce thin archives

Refactored some code and add support of U16 & U32 characters, as well as tests for them.

I have refactored my code so it should looks cleaner now, but I'm not sure how to add a test. It seems that adding a test for thin archive on macOS platforms can be not so straightforward. I see that in lldb/test/API/functionalities/archives/Makefile, the test suite is using libtool instead of ar to create archives on macOS platforms, and I don't know whether I can produce a thin archive with that. Also, ld on macOS platforms seems to be unable to identify thin archives (I'm using ld64.lld when testing my code locally).

Hi, I just forgot to mention that I don't really have access to commit... So if any people see this and this patch looks good to them, maybe please help me commit this one?

Update to using std::atomic to avoid data race instead of mutex lock.

In D118331#3298972, @aprantl wrote:

This is great, but I recently started a patch series to upstream Apple's internal implementation. Maybe we can work together to land this feature?

Please ignore my last updated diff, I didn't see int3's latest comment at that time. I will see what I could do about those two new issues, and as for this test, I think maybe I should make it conform to int3's newest revision (https://reviews.llvm.org/D112195).

Now my test uses cp command to create framework instead of ln command. Hope this will do, and if there is any problem, please let me know so I can try other workarounds.

Removed some unnecessary saver.save().

I removed the unnecessary "Function Attrs" line in my test, and wrote some comments to explain why some lines (which are seemingly not testing linker options) is added to the test.

Shorten my test by removing some irrelevant symbols.

I rewrote my test using llvm IR, and add it to the lc-linker-option.ll test. I also add "saver.save()" wherever a function calls resolveDylibPath without saving the string itself, and removed saving StringRef instance in DriverUtils.cpp:line 199.

I added %t to paths in my test, and they should be ok now. I also changed a few functions related to adding frameworks, so now they return StringRef instances instead of std::string, and their values are all saved by saver.save(). In this way I think we don't need to change the key of loadArchives to hash_code.

Add a test case to illustrate where a framework (archive) is loaded twice.