<p dir="ltr">Lgtm </p>
<div class="gmail_quote">On Jun 2, 2015 1:31 PM, "Russell Gallop" <<a href="mailto:russell.gallop@gmail.com">russell.gallop@gmail.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi rafael, echristo,<br>
<br>
Files compiled with -via-file-asm should be byte for byte identical to those compiled in the same way without that option. This change adds an exact check to check_cfc.py to spot non-code differences. If there is a difference, the check then compares code and debug to try and report more detail on the check failure.<br>
<br>
This could be helpful in finding bugs like this one: <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__llvm.org_bugs_show-5Fbug.cgi-3Fid-3D14524&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=Ohudu3oeuJNhkJtrqQsaSUGoyRVIjYDApljDqmgPjyE&s=WwOF3hM7z1mJq7DtN0aVWUbjtUYsyz9OQ2UZGgTI4_0&e=" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=14524</a><br>
<br>
Running the llvm-test-suite there are dash_s_no_change check failures (as of r238815) but not failures in check_cfc.py or new failures introduced by this change. I will report bugs for these check failures.<br>
<br>
Please let me know if okay to commit?<br>
<br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10188&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=Ohudu3oeuJNhkJtrqQsaSUGoyRVIjYDApljDqmgPjyE&s=wpUUYX4eNV6uk-k9amqwoxvCEPhcFdS1aH_BWxpfrg8&e=" target="_blank">http://reviews.llvm.org/D10188</a><br>
<br>
Files:<br>
utils/check_cfc/check_cfc.py<br>
utils/check_cfc/obj_diff.py<br>
<br>
Index: utils/check_cfc/check_cfc.py<br>
===================================================================<br>
--- utils/check_cfc/check_cfc.py<br>
+++ utils/check_cfc/check_cfc.py<br>
@@ -282,12 +282,24 @@<br>
run_step(alternate_command, my_env,<br>
"Error compiling with -via-file-asm")<br>
<br>
- # Compare disassembly (returns first diff if differs)<br>
- difference = obj_diff.compare_object_files(self._output_file_a,<br>
- output_file_b)<br>
- if difference:<br>
- raise WrapperCheckException(<br>
- "Code difference detected with -S\n{}".format(difference))<br>
+ # Compare if object files are exactly the same<br>
+ exactly_equal = obj_diff.compare_exact(self._output_file_a, output_file_b)<br>
+ if not exactly_equal:<br>
+ # Compare disassembly (returns first diff if differs)<br>
+ difference = obj_diff.compare_object_files(self._output_file_a,<br>
+ output_file_b)<br>
+ if difference:<br>
+ raise WrapperCheckException(<br>
+ "Code difference detected with -S\n{}".format(difference))<br>
+<br>
+ # Code is identical, compare debug info<br>
+ dbgdifference = obj_diff.compare_debug_info(self._output_file_a,<br>
+ output_file_b)<br>
+ if dbgdifference:<br>
+ raise WrapperCheckException(<br>
+ "Debug info difference detected with -S\n{}".format(dbgdifference))<br>
+<br>
+ raise WrapperCheckException("Object files not identical with -S\n")<br>
<br>
# Clean up temp file if comparison okay<br>
os.remove(output_file_b)<br>
Index: utils/check_cfc/obj_diff.py<br>
===================================================================<br>
--- utils/check_cfc/obj_diff.py<br>
+++ utils/check_cfc/obj_diff.py<br>
@@ -4,6 +4,7 @@<br>
<br>
import argparse<br>
import difflib<br>
+import filecmp<br>
import os<br>
import subprocess<br>
import sys<br>
@@ -26,6 +27,15 @@<br>
sys.exit(1)<br>
return filter(keep_line, out.split(os.linesep))<br>
<br>
+def dump_debug(objfile):<br>
+ """Dump all of the debug info from a file."""<br>
+ p = subprocess.Popen([disassembler, '-WliaprmfsoRt', objfile], stdout=subprocess.PIPE, stderr=subprocess.PIPE)<br>
+ (out, err) = p.communicate()<br>
+ if p.returncode or err:<br>
+ print("Dump debug failed: {}".format(objfile))<br>
+ sys.exit(1)<br>
+ return filter(keep_line, out.split(os.linesep))<br>
+<br>
def first_diff(a, b, fromfile, tofile):<br>
"""Returns the first few lines of a difference, if there is one. Python<br>
diff can be very slow with large objects and the most interesting changes<br>
@@ -63,6 +73,22 @@<br>
disb = disassemble(objfileb)<br>
return first_diff(disa, disb, objfilea, objfileb)<br>
<br>
+def compare_debug_info(objfilea, objfileb):<br>
+ """Compare debug info of two different files.<br>
+ Allowing unavoidable differences, such as filenames.<br>
+ Return the first difference if the debug info differs, or None.<br>
+ If there are differences in the code, there will almost certainly be differences in the debug info too.<br>
+ """<br>
+ dbga = dump_debug(objfilea)<br>
+ dbgb = dump_debug(objfileb)<br>
+ return first_diff(dbga, dbgb, objfilea, objfileb)<br>
+<br>
+def compare_exact(objfilea, objfileb):<br>
+ """Byte for byte comparison between object files.<br>
+ Returns True if equal, False otherwise.<br>
+ """<br>
+ return filecmp.cmp(objfilea, objfileb)<br>
+<br>
if __name__ == '__main__':<br>
parser = argparse.ArgumentParser()<br>
parser.add_argument('objfilea', nargs=1)<br>
<br>
EMAIL PREFERENCES<br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=Ohudu3oeuJNhkJtrqQsaSUGoyRVIjYDApljDqmgPjyE&s=JoBvddpaep8Wu1IGxo0SqfDDGsgI1Qdn6wr5aqunO7c&e=" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
</blockquote></div>