smeenai (Shoaib Meenai)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 19 2016, 10:21 AM (73 w, 5 d)

Recent Activity

Today

smeenai accepted D42207: libcxx: Define set_unexpected, _get_unexpected and __uncaught_exceptions without dllimport..

Seems like an oversight on their part, since these functions will be imported from VCRUNTIME*.dll for /MD. Oh well. LGTM.

Wed, Jan 17, 4:26 PM

Yesterday

smeenai accepted D42158: libcxx: Stop using private MSVC macros in the exception implementation..

LGTM.

Tue, Jan 16, 8:20 PM

Wed, Jan 10

smeenai committed rLLD322259: [ELF] Fix SysV hash tables with --no-rosegment.
[ELF] Fix SysV hash tables with --no-rosegment
Wed, Jan 10, 10:58 PM
smeenai committed rL322259: [ELF] Fix SysV hash tables with --no-rosegment.
[ELF] Fix SysV hash tables with --no-rosegment
Wed, Jan 10, 10:58 PM
smeenai closed D41928: [ELF] Fix SysV hash tables with --no-rosegment.
Wed, Jan 10, 10:58 PM
smeenai updated the diff for D41928: [ELF] Fix SysV hash tables with --no-rosegment.

Change comment to refer to existing one

Wed, Jan 10, 10:52 PM
smeenai updated the diff for D41928: [ELF] Fix SysV hash tables with --no-rosegment.

Rui's comment

Wed, Jan 10, 10:46 PM
smeenai created D41928: [ELF] Fix SysV hash tables with --no-rosegment.
Wed, Jan 10, 5:20 PM

Tue, Jan 9

smeenai added a comment to D41884: Fix thread race between SectionPiece's OutputOff and Live members.

Should this be considered for picking to the 6.0 release branch?

Tue, Jan 9, 4:29 PM

Mon, Jan 8

smeenai committed rL322061: [cmake] Use symlinks for Windows-hosted toolchains built on Unix.
[cmake] Use symlinks for Windows-hosted toolchains built on Unix
Mon, Jan 8, 11:51 PM
smeenai closed D41314: [cmake] Use symlinks for Windows-hosted toolchains built on Unix.
Mon, Jan 8, 11:51 PM
smeenai accepted D41847: [ELF] Explicit template instantiations for addFile.

LGTM, given that it just seems like an oversight.

Mon, Jan 8, 6:27 PM
smeenai committed rL322042: [ELF] Small grammar fix. NFC.
[ELF] Small grammar fix. NFC
Mon, Jan 8, 3:19 PM
smeenai committed rLLD322042: [ELF] Small grammar fix. NFC.
[ELF] Small grammar fix. NFC
Mon, Jan 8, 3:19 PM
smeenai abandoned D41368: [libc++] Ignore bogus tautologic comparison warnings.

The warning was removed from -Wall.

Mon, Jan 8, 3:11 PM
smeenai committed rL322032: [cmake] Pass CMAKE_MAKE_PROGRAM to native configure.
[cmake] Pass CMAKE_MAKE_PROGRAM to native configure
Mon, Jan 8, 1:54 PM
smeenai committed rL322026: [COFF] Delete CanExitEarly.
[COFF] Delete CanExitEarly
Mon, Jan 8, 1:16 PM
smeenai committed rLLD322026: [COFF] Delete CanExitEarly.
[COFF] Delete CanExitEarly
Mon, Jan 8, 1:16 PM
smeenai closed D41814: [COFF] Delete CanExitEarly.
Mon, Jan 8, 1:16 PM

Sun, Jan 7

smeenai added a comment to D41803: ldd::COFF: initalize ErrorHandler with CanExitEarly value.

Not your fault. I think the right thing would happen if you added a Repository when uploading the change, and I'll update the instructions to reflect that. arcanist shouldn't be necessary. It's kinda unfortunate that Phabricator allows submitting a diff without having a repository ...

Sun, Jan 7, 10:49 PM
smeenai created D41814: [COFF] Delete CanExitEarly.
Sun, Jan 7, 10:09 PM
smeenai committed rL321983: [COFF] Initalize ErrorHandler with CanExitEarly value.
[COFF] Initalize ErrorHandler with CanExitEarly value
Sun, Jan 7, 10:00 PM
smeenai committed rLLD321983: [COFF] Initalize ErrorHandler with CanExitEarly value.
[COFF] Initalize ErrorHandler with CanExitEarly value
Sun, Jan 7, 9:59 PM
smeenai closed D41803: ldd::COFF: initalize ErrorHandler with CanExitEarly value.
Sun, Jan 7, 9:59 PM
smeenai committed rLLD321982: [ELF] Drop unnecessary VersionId setting in scanShlibUndefined.
[ELF] Drop unnecessary VersionId setting in scanShlibUndefined
Sun, Jan 7, 9:56 PM
smeenai committed rL321982: [ELF] Drop unnecessary VersionId setting in scanShlibUndefined.
[ELF] Drop unnecessary VersionId setting in scanShlibUndefined
Sun, Jan 7, 9:54 PM
smeenai closed D41639: [ELF] Drop unnecessary VersionId setting in scanShlibUndefined.
Sun, Jan 7, 9:54 PM
smeenai accepted D41639: [ELF] Drop unnecessary VersionId setting in scanShlibUndefined.

LGTM'd by Rafael on the mailing list: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20180101/513595.html

Sun, Jan 7, 9:53 PM

Sat, Jan 6

smeenai added a reviewer for D41803: ldd::COFF: initalize ErrorHandler with CanExitEarly value: inglorion.

Yeah, that page mentions mailing lists to use for LLVM and clang, but doesn't mention other projects. The wording could be clearer there.

Sat, Jan 6, 3:03 PM
smeenai accepted D41803: ldd::COFF: initalize ErrorHandler with CanExitEarly value.

For future reference, the standard procedure is to add the relevant -commits mailing list (llvm-commits in this case) as a subscriber when uploading the patch, so that a mail gets sent out to the list. Over here the patch is trivial enough that I'm comfortable accepting regardless.

Sat, Jan 6, 10:31 AM

Fri, Jan 5

smeenai accepted D41645: Move scanReloc to an auxiliary function.

Seems like a straightforward NFC refactoring. LGTM.

Fri, Jan 5, 8:17 PM
smeenai accepted D41764: [libcxx] [cmake] Add a config option LIBCXX_HAS_WIN32_THREADS for enforcing win32 threads.

Can you add documentation for _LIBCPP_HAS_THREAD_API_WIN32 to docs/DesignDocs/ThreadingSupportAPI.rst? It should have been documented before, but this seems like a good opportunity to correct that.

Fri, Jan 5, 11:53 AM
smeenai added a comment to D41764: [libcxx] [cmake] Add a config option LIBCXX_HAS_WIN32_THREADS for enforcing win32 threads.

I think LIBCXX_HAS_WIN32_THREAD_API would be more consistent with the existing configuration define names?

Fri, Jan 5, 5:45 AM
smeenai added a comment to D41723: Introduce the "retpoline" x86 mitigation technique for variant #2 of the speculative execution vulnerabilities disclosed today, specifically identified by CVE-2017-5715, "Branch Target Injection", and is one of the two halves to Spectre...

Thanks! I'm however not seeing the updated commit message that contains the usage documentation of the new option in the differential revision.

Yeah, not sure why phab does't update that -- it will be updated when it lands.

Fri, Jan 5, 5:25 AM

Thu, Jan 4

smeenai updated subscribers of D41723: Introduce the "retpoline" x86 mitigation technique for variant #2 of the speculative execution vulnerabilities disclosed today, specifically identified by CVE-2017-5715, "Branch Target Injection", and is one of the two halves to Spectre...
Thu, Jan 4, 1:33 AM

Tue, Jan 2

smeenai added a comment to D41673: [CMake] Install resource files into a share/ directory.

Looks like the blacklist files are installed using add_compiler_rt_resource_file, so at the very least, this would require updating getDefaultBlacklist in clang to look in the new location?

Tue, Jan 2, 9:19 PM
smeenai added a comment to D41673: [CMake] Install resource files into a share/ directory.

I like this idea, but will anything else need to be updated if these files are installed in this new location?

Tue, Jan 2, 9:15 PM
smeenai accepted D41214: [cmake] Fix typo in test/asan/CMakeLists.txt.

The two should be equivalent unless CMAKE_SYSTEM_VERSION expands out to a value that's a variable name (which is unlikely, since it should be expanding out to a number), but this is cleaner and more consistent.

Tue, Jan 2, 9:12 PM
smeenai added a comment to D41402: [cmake] Fix DESTDIR support in compiler-rt build.

@phosek you're right that an empty CMAKE_INSTALL_PREFIX is more canonical than / though (even if it doesn't make a difference in this case). CMake strips trailing /s from CMAKE_INSTALL_PREFIX before using it, so the / ends up becoming empty anyway.

Tue, Jan 2, 11:17 AM
smeenai added a comment to D41402: [cmake] Fix DESTDIR support in compiler-rt build.

@phosek over here (as in before this patch), COMPILER_RT_INSTALL_PATH is set to CMAKE_INSTALL_PREFIX, and then we have install commands like install(... DESTINATION ${COMPILER_RT_INSTALL_PATH}). Aren't those installation commands gonna fail for an empty COMPILER_RT_INSTALL_PATH? (I'm not setup to try a compiler-rt build right now, but I did reproduce that error in a test CMake project.)

Tue, Jan 2, 11:10 AM
smeenai added a comment to D41402: [cmake] Fix DESTDIR support in compiler-rt build.

This does seem like the cleanest solution to me (though I'm not comfortable enough with compiler-rt's build system to approve this).

Tue, Jan 2, 10:58 AM
smeenai added inline comments to D41512: [Sema] -Wtautological-constant-compare is too good. Cripple it..
Tue, Jan 2, 9:54 AM · Restricted Project
smeenai added a comment to D41660: [cmake] Add new linux toolchain file.

Cache files are preferred since they are only loaded once

Tue, Jan 2, 8:39 AM
smeenai added a comment to D41660: [cmake] Add new linux toolchain file.

Why is this a cache file rather than a toolchain file (but passing itself as a toolchain file to CMake under some circumstances?) Aren't toolchain files traditionally used for cross-compilation?

Tue, Jan 2, 1:54 AM
smeenai added a comment to D41314: [cmake] Use symlinks for Windows-hosted toolchains built on Unix.

We should either go in this direction or change the installation to also create a copy rather than a symlink (but I've outlined why I prefer symlinks personally in the summary). The current situation, where the build tree has copied but the install tree has symlinks (while attempting to create copies but failing because of a CMake subtlety) is really confusing.

Tue, Jan 2, 1:22 AM

Sun, Dec 31

smeenai added a comment to D41606: [COFF] support /ignore:4217.

General question: it looks like a lot of COFF tests are written using yaml2obj, whereas ELF always uses assembly files and llvm-mc (and I've been following that pattern for the few COFF patches I've written as well). Is there an actual preference for yaml2obj tests in COFF, or is it just a historic artifact? I personally find assembly tests a lot easier to understand.

I also agree that .s tests are easier. I remember some of the tests I've added recently where I despite that ended up using yaml2obj for features that weren't quite as easy to express in .s form.

Sun, Dec 31, 6:32 AM

Sat, Dec 30

smeenai edited reviewers for D41639: [ELF] Drop unnecessary VersionId setting in scanShlibUndefined, added: espindola; removed: rafael.
Sat, Dec 30, 8:37 PM
smeenai updated subscribers of D41644: Rename --icf-data and add a corresponding flag for functions.

I don't know of a way to delete it, but a Phabricator admin might be able to help you mark it as disabled (similar to the even older @rafael.espindola account).

Sat, Dec 30, 8:36 PM
smeenai updated subscribers of D41644: Rename --icf-data and add a corresponding flag for functions.

Rafael, is the @rafael account stale now, or are you keeping both? (In other words, which one should I be adding to reviews?)

Sat, Dec 30, 7:53 PM
smeenai created D41639: [ELF] Drop unnecessary VersionId setting in scanShlibUndefined.
Sat, Dec 30, 12:39 AM
smeenai committed rLLD321578: [ELF] Only scan executables for shlib undefined symbols.
[ELF] Only scan executables for shlib undefined symbols
Sat, Dec 30, 12:03 AM
smeenai committed rL321578: [ELF] Only scan executables for shlib undefined symbols.
[ELF] Only scan executables for shlib undefined symbols
Sat, Dec 30, 12:01 AM
smeenai closed D41524: [ELF] Only scan executables for shlib undefined symbols.
Sat, Dec 30, 12:01 AM

Thu, Dec 28

smeenai added a comment to D41606: [COFF] support /ignore:4217.

General question: it looks like a lot of COFF tests are written using yaml2obj, whereas ELF always uses assembly files and llvm-mc (and I've been following that pattern for the few COFF patches I've written as well). Is there an actual preference for yaml2obj tests in COFF, or is it just a historic artifact? I personally find assembly tests a lot easier to understand.

Thu, Dec 28, 6:19 AM

Wed, Dec 27

smeenai added a comment to D41524: [ELF] Only scan executables for shlib undefined symbols.
In D41524#963992, @ruiu wrote:

LGTM, but please also fix scanShlibUndefined so that it doesn't automatically make symbols visible if they are referenced by DSOs. That seems like a root cause of the issue.

Wed, Dec 27, 12:54 PM

Thu, Dec 21

smeenai created D41524: [ELF] Only scan executables for shlib undefined symbols.
Thu, Dec 21, 3:00 PM

Tue, Dec 19

smeenai added a comment to D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent.

I posted to cfe-dev: http://lists.llvm.org/pipermail/cfe-dev/2017-December/056450.html

Tue, Dec 19, 3:03 PM · Restricted Project
smeenai added a reviewer for D41402: [cmake] Fix DESTDIR support in compiler-rt build: phosek.
Tue, Dec 19, 10:29 AM
smeenai resigned from D41402: [cmake] Fix DESTDIR support in compiler-rt build.
Tue, Dec 19, 10:28 AM
smeenai accepted D41402: [cmake] Fix DESTDIR support in compiler-rt build.

(gonna resign afterwards to clear out my request changes)

Tue, Dec 19, 10:28 AM
smeenai resigned from D41402: [cmake] Fix DESTDIR support in compiler-rt build.

Okay, looks like I was mistaken and the normalization only happens when an untyped variable from the command line gets converted to a PATH or FILEPATH variable by a set. Weird.

Tue, Dec 19, 10:28 AM
smeenai requested changes to D41402: [cmake] Fix DESTDIR support in compiler-rt build.

cmake will expand out relative paths for PATH cache variables, so the . will actually be expanded out to the build directory, which isn't desirable. You need to use the STRING type to avoid this normalization (though that's hacky).

Tue, Dec 19, 10:17 AM
smeenai added a comment to D41368: [libc++] Ignore bogus tautologic comparison warnings.

Also FWIW I agree with you that the warning is completely bogus here, but it looks like the fix to that warning isn't gonna make it into clang 6, at least, so we have to adjust accordingly.

Tue, Dec 19, 8:51 AM
smeenai added a comment to D41368: [libc++] Ignore bogus tautologic comparison warnings.

@mclow.lists are you okay with this approach? I'm also fine using a cast to silence the warning, as @zturner suggested, but we should suppress the warning in some way, otherwise libc++ 6 is gonna have compile warnings with clang 6 out of the box, which isn't great.

Tue, Dec 19, 8:50 AM

Dec 18 2017

smeenai updated the summary of D41368: [libc++] Ignore bogus tautologic comparison warnings.
Dec 18 2017, 3:09 PM
smeenai added a comment to D39149: [libc++] Prevent tautological comparisons.

I put up D41368 to just suppress the warnings instead.

Dec 18 2017, 3:09 PM
smeenai updated the diff for D41368: [libc++] Ignore bogus tautologic comparison warnings.

Remove stray comment

Dec 18 2017, 3:08 PM
smeenai created D41368: [libc++] Ignore bogus tautologic comparison warnings.
Dec 18 2017, 3:07 PM
smeenai committed rLLD321022: [ELF] Fix typo in comment. NFC.
[ELF] Fix typo in comment. NFC
Dec 18 2017, 12:34 PM
smeenai committed rL321022: [ELF] Fix typo in comment. NFC.
[ELF] Fix typo in comment. NFC
Dec 18 2017, 12:34 PM
smeenai added inline comments to D38239: [ELF] - Define linkerscript symbols early..
Dec 18 2017, 9:17 AM

Dec 15 2017

smeenai created D41314: [cmake] Use symlinks for Windows-hosted toolchains built on Unix.
Dec 15 2017, 4:52 PM
smeenai committed rLLD320896: [COFF] Clean up debug option handling.
[COFF] Clean up debug option handling
Dec 15 2017, 4:24 PM
smeenai committed rL320896: [COFF] Clean up debug option handling.
[COFF] Clean up debug option handling
Dec 15 2017, 4:24 PM
smeenai closed D41310: [COFF] Clean up debug option handling.
Dec 15 2017, 4:24 PM
smeenai added inline comments to D41310: [COFF] Clean up debug option handling.
Dec 15 2017, 4:17 PM
smeenai requested review of D41310: [COFF] Clean up debug option handling.

This has changed significantly since the last iteration.

Dec 15 2017, 4:05 PM
smeenai updated the diff for D41310: [COFF] Clean up debug option handling.

Update after discussion

Dec 15 2017, 4:05 PM
smeenai committed rL320894: [COFF] Update an outdated comment. NFC.
[COFF] Update an outdated comment. NFC
Dec 15 2017, 3:53 PM
smeenai committed rLLD320894: [COFF] Update an outdated comment. NFC.
[COFF] Update an outdated comment. NFC
Dec 15 2017, 3:53 PM
smeenai committed rLLD320892: [COFF] Simplify hasArgs calls. NFC.
[COFF] Simplify hasArgs calls. NFC
Dec 15 2017, 3:52 PM
smeenai committed rL320892: [COFF] Simplify hasArgs calls. NFC.
[COFF] Simplify hasArgs calls. NFC
Dec 15 2017, 3:52 PM
smeenai added a comment to D41310: [COFF] Clean up debug option handling.

So it looks like clang produces both CodeView and DWARF in the same object file if you pass both -gcodeview and -gdwarf. I haven't verified the debug info is sensible, but the object file at least contains both CodeView and DWARF debug sections.

Dec 15 2017, 2:05 PM
smeenai added a comment to D41310: [COFF] Clean up debug option handling.
In D41310#957239, @ruiu wrote:

Conceptually, an executable that has both DAWRF and PDB debug info is valid, no?

Dec 15 2017, 1:48 PM
smeenai added a comment to D41310: [COFF] Clean up debug option handling.

@zturner I do agree that /debug and /debug:dwarf is a bit confusing. We have to keep /debug for link.exe compatibility, so perhaps /debug:dwarf could be renamed to something else? Any suggestions?

Dec 15 2017, 1:46 PM
smeenai created D41310: [COFF] Clean up debug option handling.
Dec 15 2017, 1:40 PM
smeenai committed rL320793: Repair Windows buildbots after r320792.
Repair Windows buildbots after r320792
Dec 15 2017, 12:09 AM
smeenai committed rLLD320793: Repair Windows buildbots after r320792.
Repair Windows buildbots after r320792
Dec 15 2017, 12:09 AM

Dec 14 2017

smeenai committed rLLD320792: [COFF] Warn for locally imported symbols.
[COFF] Warn for locally imported symbols
Dec 14 2017, 11:50 PM
smeenai committed rL320792: [COFF] Warn for locally imported symbols.
[COFF] Warn for locally imported symbols
Dec 14 2017, 11:50 PM
smeenai closed D41269: [COFF] Warn for locally imported symbols.
Dec 14 2017, 11:50 PM
smeenai added inline comments to D41269: [COFF] Warn for locally imported symbols.
Dec 14 2017, 8:49 PM
smeenai updated the diff for D41269: [COFF] Warn for locally imported symbols.

Use [] for dictionary insertion

Dec 14 2017, 8:49 PM
smeenai updated the diff for D41269: [COFF] Warn for locally imported symbols.

Rui's comments; add test for multiple input files

Dec 14 2017, 8:36 PM
smeenai added inline comments to D41269: [COFF] Warn for locally imported symbols.
Dec 14 2017, 7:56 PM
smeenai added inline comments to D41269: [COFF] Warn for locally imported symbols.
Dec 14 2017, 7:01 PM
smeenai created D41269: [COFF] Warn for locally imported symbols.
Dec 14 2017, 6:52 PM
smeenai committed rL320785: [cmake] Fix clang-cl cross-compilation on macOS.
[cmake] Fix clang-cl cross-compilation on macOS
Dec 14 2017, 5:06 PM
smeenai closed D41219: [cmake] Fix clang-cl cross-compilation on macOS.
Dec 14 2017, 5:06 PM
smeenai committed rL320724: [cmake] Only attempt to install MSVC system libraries on Windows.
[cmake] Only attempt to install MSVC system libraries on Windows
Dec 14 2017, 10:42 AM