This is an archive of the discontinued LLVM Phabricator instance.

[Support] MemoryBlock size should reflect the requested size
ClosedPublic

Authored by andrewng on Mar 21 2019, 12:20 PM.

Details

Summary

This patch mirrors the change made to the Unix equivalent in
r351916. This in turn fixes bugs related to the use of FileOutputBuffer
to output to "-", i.e. stdout, on Windows.

Diff Detail

Repository
rL LLVM

Event Timeline

andrewng created this revision.Mar 21 2019, 12:20 PM
ruiu added a comment.Mar 21 2019, 1:00 PM

I'm wondering why it wasn't caught by a Windows bot if I missed it for Windows. Could you write a test that fails currently and passes after this change?

rnk added a subscriber: thakis.Mar 21 2019, 3:57 PM

I'm wondering why it wasn't caught by a Windows bot if I missed it for Windows. Could you write a test that fails currently and passes after this change?

The stdout.s test you added in fact failed on Windows because this change was missing, and @thakis disabled it in rL351949. So, I think we can just re-enable stdout.s to test this.

The stdout.s test you added in fact failed on Windows because this change was missing, and @thakis disabled it in rL351949. So, I think we can just re-enable stdout.s to test this.

Yes, I was going to re-enable this test in a separate patch (as it's in LLD) and perhaps revert the changes to the WASM tests too. It was these WASM test failures that got me looking into this issue in the first place.

I've added D59824 to restore testing and usage of output to "-".

ruiu accepted this revision.Mar 26 2019, 9:34 AM

LGTM

This revision is now accepted and ready to land.Mar 26 2019, 9:34 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2019, 3:25 AM
Herald added a subscriber: kristina. · View Herald Transcript