Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extractFile() uses hardcoded "ar" executable #31

Closed
arrowd opened this issue Oct 22, 2019 · 14 comments
Closed

extractFile() uses hardcoded "ar" executable #31

arrowd opened this issue Oct 22, 2019 · 14 comments

Comments

@arrowd
Copy link
Contributor

arrowd commented Oct 22, 2019

FreeBSD has pretty old ar in the base system, so we need to use custom ar. However, extractFile() function in extractor.go doesn't take in account command line or environment overrides.

@ianamason
Copy link
Member

Thanks @arrowd, I will fix this today.

@ianamason
Copy link
Member

@arrowd, so I added a switch

Usage of get-bc:
  -a string
    	the llvm archiver (i.e. llvm-ar)
  -b	build a bitcode module
  -l string
    	the llvm linker (i.e. llvm-link)
  -m	write the manifest
  -n int
    	maximum llvm-link command line size (in bytes)
  -o string
    	the output file
  -r string
    	the system archiver (i.e. ar)
  -s	sort the bitcode files
  -t	keep temporary linking folder
  -v	verbose mode

You want the new -r switch. I haven't tested it yet. Let me know if its totally broken.

@arrowd
Copy link
Contributor Author

arrowd commented Oct 23, 2019

That did helped a bit. It now uses correct ar executable, but fails down road with

DEBUG:defaultPath = llvm-ar
DEBUG:envPath = 
DEBUG:usrPath = llvm-ar90
DEBUG:path = llvm-ar90
DEBUG:defaultPath = llvm-link
DEBUG:envPath = 
DEBUG:usrPath = llvm-link90
DEBUG:path = llvm-link90
INFO:ea.Verbose: false
INFO:ea.WriteManifest: false
INFO:ea.BuildBitcodeModule: false
INFO:ea.LlvmArchiverName: llvm-ar90
INFO:ea.LinkerName: llvm-link90
INFO:ea.OutputFile: 
INFO:ea.InputFile: a.out
INFO:ea.InputFile real path: a.out
INFO:ea.LinkArgSize 0
INFO:ea.KeepTemp false
WARNING:Error reading the .llvm_bc section of ELF file a.out.
INFO:handleExecutable: artifactPaths = []
INFO:Calling [get-bc -a llvm-ar90 -l llvm-link90 -r llvm-ar90 a.out] DID NOT TELL US WHAT HAPPENED

@ianamason
Copy link
Member

So I need to know a bit more about a.out to debug this. How was it generated? When I mimic your line with say

get-bc -a llvm-ar -l llvm-link -r llvm-ar -o node.bc ./node-v10.16.0/node

after building nodejs (I just added that as a stress test example) I get my node.bc file just fiine.

@arrowd
Copy link
Contributor Author

arrowd commented Oct 23, 2019

I created a simple C program

int main(int argc, char* argv[])
{
return 0;
}

and compiled it using gclang test.c. On resulting binary I ran get-bc a.out and got the same error message:

WARNING:Error reading the .llvm_bc section of ELF file a.out.

Running readelf -S a.out confirms that there is no suck section at all.

Could it be that gclang also invokes some toolchain utility, which is slightly incompatible on FreeBSD?

@ianamason
Copy link
Member

OK so it helps that it is a trivial example. Yes on *nix we use objcopy to add the section, the action takes place in:

func attachBitcodePathToObject(bcFile, objFile string)

So you could see what happens when you try that manually.
Generate a.out with clang, in the same directory as where you did the gclang (so there will be a
file called .trivial.c.o.bc as well as .trivial.c.o (my version of your simple program is trivial.c)

Then you can do:

objcopy --add-section .llvm_bc=tempfile .trivial.c.o

where tempfile just contains the path to the .trivial.c.o.bc

But before you do that though try it with the lastest because I changed a LogDebug to a LogWarning.

@ianamason
Copy link
Member

P.S. I am going offline for an hour...

@arrowd
Copy link
Contributor Author

arrowd commented Oct 23, 2019

Ok, this is super strange, but I did chased it down.

FreeBSD's objcopy is elftooclhain one. Running objcopy --add-section .llvm_bc=.test.c.o.bc .test.c.o first time it does nothing. However, running the same command again actually adds the section! It's clearly a bug on our side, and I'll report it upstream.

For now, would it be possible to add another override to objcopy executable being used?

@ianamason
Copy link
Member

Sure, that's on the compile side, so it will have to be (another) environment variable. Shouldn't take long.

@ianamason
Copy link
Member

bce2126 has the update. You're my tester @arrowd

@arrowd
Copy link
Contributor Author

arrowd commented Oct 23, 2019

Yay, it works! Thanks for fixing this.

Now, the last thing I'd ask is to make a new release, so that we can update our package: https://www.freshports.org/devel/gllvm

@ianamason
Copy link
Member

Sure. When I get back from lunch.

@ianamason
Copy link
Member

v1.2.5 is live.

@arrowd
Copy link
Contributor Author

arrowd commented Oct 23, 2019

Great, thanks for your prompt support.

@arrowd arrowd closed this as completed Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants