Page MenuHomePhabricator

ecbeckmann (Eric Beckmann)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 14 2017, 1:41 PM (126 w, 5 d)

Recent Activity

Mar 30 2018

ecbeckmann accepted D45106: [Build] Use LIBXML2_LIBRARIES from find_package.
Mar 30 2018, 2:31 PM · Restricted Project

Nov 10 2017

ecbeckmann accepted D39892: [llvm-cvtres] Add support for ARM64.
Nov 10 2017, 12:47 PM

Oct 16 2017

ecbeckmann accepted D38974: COFF: Add resource files to linkrepro instead of the cvtres object file..

This looks like a good improvement for clarity. Should wait for Rui's approval before submitting.

Oct 16 2017, 2:42 PM

Oct 13 2017

ecbeckmann accepted D38875: Fix the incorrect detection of ICONV_LIBRARY_PATH.
Oct 13 2017, 5:10 PM

Sep 15 2017

ecbeckmann committed rL313431: Revert "Fix Bug 30978 by emitting cv file checksums.".
Revert "Fix Bug 30978 by emitting cv file checksums."
Sep 15 2017, 6:16 PM
ecbeckmann committed rL313374: Fix Bug 30978 by emitting cv file checksums..
Fix Bug 30978 by emitting cv file checksums.
Sep 15 2017, 11:22 AM
ecbeckmann closed D37157: Fix Bug 30978 by emitting cv file checksums. by committing rL313374: Fix Bug 30978 by emitting cv file checksums..
Sep 15 2017, 11:22 AM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Updated comment formatting.

Sep 15 2017, 11:21 AM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Changed some comments.

Sep 15 2017, 11:14 AM
ecbeckmann added inline comments to D37157: Fix Bug 30978 by emitting cv file checksums..
Sep 15 2017, 10:56 AM

Sep 14 2017

ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Removed some last unnecessary changes.

Sep 14 2017, 7:55 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Some formatting fixes.

Sep 14 2017, 7:52 PM
ecbeckmann added inline comments to D37157: Fix Bug 30978 by emitting cv file checksums..
Sep 14 2017, 7:36 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Don't leak symbol into assembler, remove enum class, use createTempSymbol.

Sep 14 2017, 7:36 PM
ecbeckmann committed rL313312: Fix bug 34608 by moving private header out of public header..
Fix bug 34608 by moving private header out of public header.
Sep 14 2017, 4:02 PM
ecbeckmann closed D37863: Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead. by committing rL313312: Fix bug 34608 by moving private header out of public header..
Sep 14 2017, 4:02 PM
ecbeckmann added inline comments to D37863: Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead..
Sep 14 2017, 4:00 PM
ecbeckmann updated the diff for D37863: Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead..

Put synonymous header first.

Sep 14 2017, 4:00 PM
ecbeckmann added inline comments to D37157: Fix Bug 30978 by emitting cv file checksums..
Sep 14 2017, 2:26 PM
ecbeckmann added reviewers for D37157: Fix Bug 30978 by emitting cv file checksums.: zturner, rnk.
Sep 14 2017, 1:49 PM
ecbeckmann added inline comments to D37157: Fix Bug 30978 by emitting cv file checksums..
Sep 14 2017, 1:48 PM
ecbeckmann changed the visibility for D37157: Fix Bug 30978 by emitting cv file checksums..
Sep 14 2017, 1:48 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Reformat.

Sep 14 2017, 1:44 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Reformat

Sep 14 2017, 1:22 PM
ecbeckmann added reviewers for D37863: Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead.: orivej, zturner.
Sep 14 2017, 1:06 PM
ecbeckmann changed the visibility for D37863: Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead..
Sep 14 2017, 1:06 PM
ecbeckmann updated the diff for D37863: Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead..

Exclude checksums

Sep 14 2017, 1:05 PM
ecbeckmann retitled D37863: Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead. from Fix bug 34608 by moving private header out of public header. WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead. to Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead..
Sep 14 2017, 1:02 PM
ecbeckmann created D37863: Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead..
Sep 14 2017, 1:01 PM
ecbeckmann added a comment to D37841: [llvm-rc] Refactoring needed for ACCELERATORS and MENU resources serialization (serialization, pt 1.5)..

This looks good to me.

Sep 14 2017, 11:35 AM

Sep 13 2017

ecbeckmann changed the visibility for D37157: Fix Bug 30978 by emitting cv file checksums..
Sep 13 2017, 6:46 PM
ecbeckmann added inline comments to D37157: Fix Bug 30978 by emitting cv file checksums..
Sep 13 2017, 6:45 PM
ecbeckmann changed the visibility for D37157: Fix Bug 30978 by emitting cv file checksums..
Sep 13 2017, 6:45 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Format

Sep 13 2017, 5:31 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Remove errant errors.

Sep 13 2017, 5:14 PM
ecbeckmann added inline comments to D37828: [llvm-rc] Serialize MENU resources to .res files (serialization, pt 3)..
Sep 13 2017, 3:42 PM
ecbeckmann added inline comments to D37824: [llvm-rc] Serialize ACCELERATORS resources to .res files (serialization, pt 2)..
Sep 13 2017, 2:22 PM

Sep 12 2017

ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Restructrue, create real symbol not temp.

Sep 12 2017, 5:59 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Add checksum offset directive.

Sep 12 2017, 10:47 AM

Sep 9 2017

ecbeckmann added a comment to D37157: Fix Bug 30978 by emitting cv file checksums..

Why do we have to add placeholders for the checksum offsets? Aren't they just added in order, one after another?

Sep 9 2017, 12:04 PM
ecbeckmann added a comment to D37157: Fix Bug 30978 by emitting cv file checksums..

Fortunately this seems to work whether generating assembly or object

Sep 9 2017, 11:51 AM
ecbeckmann added a comment to D37157: Fix Bug 30978 by emitting cv file checksums..

Make this entire thing easier to follow and understand.

Sep 9 2017, 11:51 AM

Sep 8 2017

ecbeckmann added inline comments to D37157: Fix Bug 30978 by emitting cv file checksums..
Sep 8 2017, 7:26 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Everything works finally.

Sep 8 2017, 7:00 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Imprved?

Sep 8 2017, 4:19 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Use MCFixup

Sep 8 2017, 11:44 AM

Sep 7 2017

ecbeckmann removed reviewers for D37157: Fix Bug 30978 by emitting cv file checksums.: zturner, rnk.
Sep 7 2017, 10:51 AM
ecbeckmann changed the visibility for D37157: Fix Bug 30978 by emitting cv file checksums..
Sep 7 2017, 10:51 AM

Sep 6 2017

ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Some more testing changes.

Sep 6 2017, 7:01 PM
ecbeckmann added inline comments to D37240: Fix crbug 759265 by suppressing llvm mt warnings..
Sep 6 2017, 11:07 AM
ecbeckmann added inline comments to D37240: Fix crbug 759265 by suppressing llvm mt warnings..
Sep 6 2017, 11:06 AM

Sep 5 2017

ecbeckmann committed rL312604: Fix crbug 759265 by suppressing llvm mt warnings..
Fix crbug 759265 by suppressing llvm mt warnings.
Sep 5 2017, 6:51 PM
ecbeckmann closed D37240: Fix crbug 759265 by suppressing llvm mt warnings. by committing rL312604: Fix crbug 759265 by suppressing llvm mt warnings..
Sep 5 2017, 6:51 PM
ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Add line to end of file.

Sep 5 2017, 6:49 PM
ecbeckmann added inline comments to D37240: Fix crbug 759265 by suppressing llvm mt warnings..
Sep 5 2017, 6:25 PM
ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

tests

Sep 5 2017, 5:44 PM
ecbeckmann added inline comments to D37240: Fix crbug 759265 by suppressing llvm mt warnings..
Sep 5 2017, 5:33 PM
ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Use stringref

Sep 5 2017, 5:33 PM
ecbeckmann added inline comments to D37240: Fix crbug 759265 by suppressing llvm mt warnings..
Sep 5 2017, 4:15 PM
ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

return string instead of buffer

Sep 5 2017, 4:15 PM
ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Make isAvailable() function instead of using flag.

Sep 5 2017, 3:51 PM
ecbeckmann added a comment to D37240: Fix crbug 759265 by suppressing llvm mt warnings..
In D37240#861561, @ruiu wrote:

Hmm, now it doesn't look like it is towards a right direction.

As I understand the API of your WindowsManifestMerger class, it provides an empty implementation if libxml2 is not available so any code that uses your WindowsManifestMerger class can compile at least on any condition (though it may raise an error/returns an empty result at runtime if it wasn't compiled with libxml2). This patch uses conditional compilation to avoid compiler errors, which seems a violation of the rule.

Sep 5 2017, 3:12 PM
ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Have internal lld functions return result directly and handle errors themselves.

Sep 5 2017, 3:07 PM
ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Sorry, previous patch did not compile and method of assigning config flag was incorrect.

Sep 5 2017, 2:55 PM
ecbeckmann added a comment to D37321: llvm-mt: Fix memory management in WindowsManifestMergerImpl::getMergedManifest.

This seems better than using XML_PARSE_NODICT, because we also address the issue of any other memory I might have failed to copy....I could do this in a subsequent patch.

So I'll submit the patch?
I'll try to enable this on our asan bot to detect problems in future.

Sep 5 2017, 11:47 AM
ecbeckmann added a comment to D37240: Fix crbug 759265 by suppressing llvm mt warnings..

We have similar issues in the PDB code, where we might want to use the native pdb reader or the DIA reader. To handle this, we have an enumeration:

/// Specifies which PDB reader implementation is to be used.  Only a value
/// of PDB_ReaderType::DIA is currently supported, but Native is in the works.
enum class PDB_ReaderType {
  DIA = 0,
  Native = 1,
};

Then, when you actually call the function to load the PDB, it looks like this:

  // Create the correct concrete instance type based on the value of Type.
  if (Type == PDB_ReaderType::Native)
    return NativeSession::createFromPdb(Path, Session);

#if LLVM_ENABLE_DIA_SDK
  return DIASession::createFromPdb(Path, Session);
#else
  return make_error<GenericError>("DIA is not installed on the system");
#endif

Then the user could write something like:

if (auto EC = loadDataForPDB(PDB_ReaderType::DIA, Session)) {
  if (auto EC2 = loadDataForPDB(PDB_ReaderType::Native, Session)) {
    return make_error<Foo>("PDB Unsupported");
  }
}

// Now I have either a native session or a DIA session, but I don't know (or care) which.
return Session;
Sep 5 2017, 10:29 AM
ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Add flag for manifest library being enabled.

Sep 5 2017, 10:25 AM

Sep 1 2017

ecbeckmann added a comment to D37321: llvm-mt: Fix memory management in WindowsManifestMergerImpl::getMergedManifest.

Sorry for being so late to respond to this, I've been busy with team match this week.

Thanks for catching these errors. It was my fault for just using unique_ptr instead of explicitly calling xmlFree on the xmlDoc structures. However, I'm confused about the XML_PARSE_NODICT flag, how will this help? As far as I can tell it prevents the creation of a new string dictionary for that xmlDoc that is parsed? Which could be a problem if I had href's and namespaces in one tree point to another. However I never do this and always duplicate the entire string from one tree to another.

As soon as I start to delete OutputDoc correctly xmlFreeDoc(Doc) in ~WindowsManifestMergerImpl() fails. I guess I see this both with or without asan. XML_PARSE_NODICT helps.

Also out of curiosity how did you discover the presence of memory problems? I've looked on msan and nothing seems to be there.

check-llvm under msan disables libxml (and other 3rd party deps) as msan needs them instrumented as well.
I am using check-llvm with asan

Sep 1 2017, 3:05 PM
ecbeckmann added inline comments to D37240: Fix crbug 759265 by suppressing llvm mt warnings..
Sep 1 2017, 2:12 PM
ecbeckmann added a comment to D37321: llvm-mt: Fix memory management in WindowsManifestMergerImpl::getMergedManifest.

Sorry for being so late to respond to this, I've been busy with team match this week.

Sep 1 2017, 2:10 PM
ecbeckmann added inline comments to D37283: [llvm-rc] Serialize HTML resources to .res files (serialization, pt 1)..
Sep 1 2017, 11:36 AM

Aug 30 2017

ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Remove one more consume.

Aug 30 2017, 4:38 PM
ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Remove unnecessary consume because toString consumes error.

Aug 30 2017, 4:35 PM
ecbeckmann added inline comments to D37240: Fix crbug 759265 by suppressing llvm mt warnings..
Aug 30 2017, 4:27 PM
ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Throw errors that internal mt has if they are not related to lack of libxml2.

Aug 30 2017, 4:25 PM

Aug 29 2017

ecbeckmann added inline comments to D37283: [llvm-rc] Serialize HTML resources to .res files (serialization, pt 1)..
Aug 29 2017, 7:13 PM
ecbeckmann added inline comments to D37240: Fix crbug 759265 by suppressing llvm mt warnings..
Aug 29 2017, 6:44 PM

Aug 28 2017

ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Remove unnecessary error move assignment.

Aug 28 2017, 6:41 PM
ecbeckmann added reviewers for D37240: Fix crbug 759265 by suppressing llvm mt warnings.: ruiu, zturner.
Aug 28 2017, 6:41 PM
ecbeckmann updated the diff for D37240: Fix crbug 759265 by suppressing llvm mt warnings..

Add -allow-empty

Aug 28 2017, 6:29 PM
ecbeckmann created D37240: Fix crbug 759265 by suppressing llvm mt warnings..
Aug 28 2017, 5:58 PM

Aug 25 2017

ecbeckmann updated the diff for D37157: Fix Bug 30978 by emitting cv file checksums..

Rebase

Aug 25 2017, 11:43 AM
ecbeckmann created D37157: Fix Bug 30978 by emitting cv file checksums..
Aug 25 2017, 11:37 AM

Aug 23 2017

ecbeckmann committed rL311625: Fix bug 34051 by handling empty .res files gracefully..
Fix bug 34051 by handling empty .res files gracefully.
Aug 23 2017, 7:38 PM
ecbeckmann closed D37044: Fix bug 34051 by handling empty .res files gracefully. by committing rL311625: Fix bug 34051 by handling empty .res files gracefully..
Aug 23 2017, 7:37 PM
ecbeckmann updated the diff for D37044: Fix bug 34051 by handling empty .res files gracefully..

Add comment

Aug 23 2017, 7:37 PM
ecbeckmann added a comment to D36891: [llvm-rc] Add ICON and HTML parsing ability [2/8].

Looks fine to me.

Aug 23 2017, 3:50 PM
ecbeckmann added inline comments to D36905: [llvm-rc] Add DIALOG(EX) parsing ability. [5/8].
Aug 23 2017, 1:40 PM
ecbeckmann updated the diff for D37044: Fix bug 34051 by handling empty .res files gracefully..

Make factory function instead of having constructor fail.

Aug 23 2017, 11:22 AM
ecbeckmann added inline comments to D37044: Fix bug 34051 by handling empty .res files gracefully..
Aug 23 2017, 11:22 AM

Aug 22 2017

ecbeckmann added a reviewer for D37044: Fix bug 34051 by handling empty .res files gracefully.: ruiu.
Aug 22 2017, 6:12 PM
ecbeckmann added reviewers for D37044: Fix bug 34051 by handling empty .res files gracefully.: zturner, thakis.
Aug 22 2017, 5:46 PM
ecbeckmann updated the diff for D37044: Fix bug 34051 by handling empty .res files gracefully..

Remove unnecessary braces

Aug 22 2017, 5:42 PM
ecbeckmann created D37044: Fix bug 34051 by handling empty .res files gracefully..
Aug 22 2017, 5:39 PM
ecbeckmann added a comment to D37033: [llvm-rc] Add user-defined resources parsing ability. [8/8].

Looks fine to me.

Aug 22 2017, 4:05 PM
ecbeckmann added inline comments to D37021: [llvm-rc] Add VERSIONINFO parsing ability. [6/8].
Aug 22 2017, 2:46 PM
ecbeckmann added a comment to D36898: [llvm-rc] Add MENU parsing ability. [4/8].

This looks fine now.

Aug 22 2017, 2:44 PM
ecbeckmann added a comment to D37022: [llvm-rc] Add integer expressions parsing ability. [7/8].

This looks fine to me.

Aug 22 2017, 2:38 PM

Aug 21 2017

ecbeckmann committed rL311424: Integrate manifest merging library into LLD..
Integrate manifest merging library into LLD.
Aug 21 2017, 8:16 PM
ecbeckmann closed D36255: Integrate manifest merging library into LLD. by committing rL311424: Integrate manifest merging library into LLD..
Aug 21 2017, 8:16 PM