Skip to content
This repository has been archived by the owner on Feb 4, 2024. It is now read-only.

Add the integer multiplication overflow check again & fix make's std output #3

Merged
merged 1 commit into from Sep 19, 2021

Conversation

bozhodimitrov
Copy link
Contributor

:)

make0x.py Outdated
@@ -13,7 +13,7 @@ def output_on_failure(cmd, cwd):
)
if ret.returncode:
raise AssertionError('Command {!r} returned {}!\nOutput:\n{}'.format(
cmd, ret.returncode, ret.stdout,
cmd, ret.returncode, ret.stdout.decode(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skipping the decode here was ~partially intentional (non-UTF-8 code would crash while producing an error message instead of showing the error message)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the error output was really unreadable single line -- example:

b"long error message            without new lines and with             white spacing all over the place"

This is why I added the decode - not sure if there is a better way to fix the output.
Maybe trying to decode and if that fails, just use ret.stdout?

make0x.py Outdated
cwd=cwd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
universal_newlines=True,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe like this? Using universal_newlines instead of text for better backwards compatibility (lol) :D

@bozhodimitrov
Copy link
Contributor Author

BTW😂:

image

make.py Outdated
Comment on lines 14 to 16
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
universal_newlines=True,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

universal_newlines has the same problem as .decode() or encoding=... or text=True in that it hides the actual error on a decoding issue

@asottile
Copy link
Member

BTW😂:

image

I am amazed that works! wow!

@asottile
Copy link
Member

I think check=True is actually the easiest patch

make0x.py Outdated
Comment on lines 15 to 21
try:
stdout = ret.stdout.decode()
except UnicodeError:
stdout = ret.stdout

raise AssertionError('Command {!r} returned {}!\nOutput:\n{}'.format(
cmd, ret.returncode, ret.stdout,
cmd, ret.returncode, stdout,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my last bet :) But if you don't like it, I will revert.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CalledProcessError exception also has the same problem with the stdout bytes attribute.

This is the output:

b'cc    -c -o intobject.o intobject.c\nIn file included from allobjects.h:48,\n                 from intobject.c:27:\nmalloc.h:59:13: warning: conflicting types for built-in function \xe2\x80\x98malloc\xe2\x80\x99; expected \xe2\x80\x98void *(long unsigned int)\xe2\x80\x99 [-Wbuiltin-declaration-mismatch]\n   59 | extern ANY *malloc PROTO((unsigned int));\n      |             ^~~~~~\nmalloc.h:1:1: note: \xe2\x80\x98malloc\xe2\x80\x99 is declared in header \xe2\x80\x98<stdlib.h>\xe2\x80\x99\n  +++ |+#include <stdlib.h>\n    1 | /***********************************************************\nmalloc.h:60:13: warning: conflicting types for built-in function \xe2\x80\x98calloc\xe2\x80\x99; expected \xe2\x80\x98void *(long unsigned int,  long unsigned int)\xe2\x80\x99 [-Wbuiltin-declaration-mismatch]\n   60 | extern ANY *calloc PROTO((unsigned int, unsigned int));\n      |             ^~~~~~\nmalloc.h:60:13: note: \xe2\x80\x98calloc\xe2\x80\x99 is declared in header \xe2\x80\x98<stdlib.h>\xe2\x80\x99\nmalloc.h:61:13: warning: conflicting types for built-in function \xe2\x80\x98realloc\xe2\x80\x99; expected \xe2\x80\x98void *(void *, long unsigned int)\xe2\x80\x99 [-Wbuiltin-declaration-mismatch]\n   61 | extern ANY *realloc PROTO((ANY *, unsigned int));\n      |             ^~~~~~~\nmalloc.h:61:13: note: \xe2\x80\x98realloc\xe2\x80\x99 is declared in header \xe2\x80\x98<stdlib.h>\xe2\x80\x99\nmalloc.h:62:13: warning: conflicting types for built-in function \xe2\x80\x98free\xe2\x80\x99; expected \xe2\x80\x98void(void *)\xe2\x80\x99 [-Wbuiltin-declaration-mismatch]\n   62 | extern void free PROTO((ANY *)); /* XXX sometimes int on Unix old systems */\n      |             ^~~~\nmalloc.h:62:13: note: \xe2\x80\x98free\xe2\x80\x99 is declared in header \xe2\x80\x98<stdlib.h>\xe2\x80\x99\nintobject.c: In function \xe2\x80\x98int_mul\xe2\x80\x99:\nintobject.c:198:8: error: lvalue required as decrement operand\n  198 |        ---x = a * b;\n      |        ^~\nmake: *** [<builtin>: intobject.o] Error 1\n'

@asottile asottile merged commit 5a19c01 into asottile-archive:master Sep 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants