extractor: Make extraction errors fatal by woodruffw · Pull Request #37 · SRI-CS...

Copy link

Contributor

ianamason commented on Aug 3, 2020

@woodruffw seems like a reasonable improvement, but the CI is still complaining about shadowing.
Are you gonna fix that or should I?

woodruffw commented on Aug 3, 2020

…

ianamason commented on Aug 4, 2020

I can wait. There is no rush. How is the other issue coming along? Weren't you guys working on issue #34 ?

woodruffw commented on Aug 4, 2020

How is the other issue coming along? Weren't you guys working on issue #34 ?

Yep, we were. We ended up taking another approach for capturing the command-line flags, so we haven't made a ton of progress on building it into gllvm itself. It's something that I can take another look at when I have some free time, though.

woodruffw commented on Aug 4, 2020

Fixed the shadowing warnings; will try to debug this:

cd libtecla; get-bc enhance; mv enhance.bc ../
mv: cannot stat ‘enhance.bc’: No such file or directory
make: *** [enhance.bc] Error 1
The command "./.travis/test.sh" exited with 2.

woodruffw commented on Aug 4, 2020

Alright, this should be good for review now (apologies for the churn, I'm not much of a Go programmer...).

The CI is failing with this error:

get-bc -b ./lib/libc.a
ERROR:Error reading the .llvm_bc section of ELF file clone.o.

...but this is to be expected, since clone.o comes from an assembly translation unit for which there can be no bitcode:

gclang -std=c99 -nostdinc -ffreestanding -Wa,--noexecstack -D_XOPEN_SOURCE=700 -I./arch/LLVM -I./arch/generic -Iobj/src/internal -I./src/include -I./src/internal -Iobj/include -I./include  -Os -pipe -fomit-frame-pointer -fno-unwind-tables -fno-asynchronous-unwind-tables -ffunction-sections -fdata-sections -Werror=implicit-function-declaration -Werror=implicit-int -Werror=pointer-sign -Werror=pointer-arith -Qunused-arguments  -c -o obj/src/thread/LLVM/clone.o src/thread/LLVM/clone.s

So this is more of a design question: should gllvm fail when the extracted/combined bitcode can't accurately reflect the original build?

ianamason commented on Aug 4, 2020

I think this is why we just warn when a segment is not found, because there is not much we can do, but we can
still forge ahead, letting the user know about the missing stuff.

"Don't let perfect be the enemy of good" is the design principle I think.

Now your version will no longer produce a final (less than perfect) bitcode result? Maybe this strictness would better be enabled by a command line switch? The default would then be the relaxed mode?

What is your motivation for this "strict" mode?

woodruffw commented on Aug 4, 2020

Now your version will no longer produce a final (less than perfect) bitcode result? Maybe this strictness would better be enabled by a command line switch? The default would then be the relaxed mode?

What is your motivation for this "strict" mode?

Yep, exactly. It currently bails out instead of producing an (incomplete) unified bitcode module. Enabling this via a command-line switch sounds good to me. I can do that in a bit.

My motivation is a static analysis ecosystem that does semi-automated builds of target programs. This strict mode makes it easier to catch eventual problems with (re)compiling the bitcode due to missing symbols (i.e. the ones present in assembly TUs).

ianamason commented on Aug 4, 2020

Yes it makes sense in the semi automated setting. Our other design principle is to be a drop in replacement for wllvm.
Warts and all.

ianamason commented on Aug 4, 2020

Thanks. If you can I would like to hear more about the "We ended up taking another approach for capturing the command-line flags" saga. Doesn't have to be here, could be there: [email protected]

woodruffw commented on Aug 4, 2020

Thanks for the merge! I'll see what I can share about the command-line flag capturing.

Would it be possible to get a quick release cut for these changes?

ianamason commented on Aug 4, 2020

Sure I will make a release this morning.

ianamason commented on Aug 4, 2020

v1.2.7 is live.

woodruffw commented on Aug 4, 2020

Thanks!