Page MenuHomePhabricator

dmikulin (Dmitry Mikulin)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 14 2016, 2:33 PM (139 w, 3 d)

Recent Activity

Apr 23 2019

dmikulin committed rCRT359043: The error message for mismatched value sites is very cryptic..
The error message for mismatched value sites is very cryptic.
Apr 23 2019, 3:43 PM
dmikulin committed rG312b5f86b7b2: The error message for mismatched value sites is very cryptic. Make it more… (authored by dmikulin).
The error message for mismatched value sites is very cryptic. Make it more…
Apr 23 2019, 3:25 PM
dmikulin committed rL359043: The error message for mismatched value sites is very cryptic..
The error message for mismatched value sites is very cryptic.
Apr 23 2019, 3:25 PM
dmikulin closed D60896: Improve error reporting for mismatched value sites in IPGO.
Apr 23 2019, 3:25 PM · Restricted Project

Apr 18 2019

dmikulin added a reviewer for D60896: Improve error reporting for mismatched value sites in IPGO: probinson.
Apr 18 2019, 4:32 PM · Restricted Project
dmikulin created D60896: Improve error reporting for mismatched value sites in IPGO.
Apr 18 2019, 4:30 PM · Restricted Project

Jun 5 2018

dmikulin added a comment to D41474: Fix a crash in lazy loading of Metadata in ThinLTO.

Any chance this gets fixed soon? I'm hitting the same problem.

Jun 5 2018, 6:03 PM

Jun 4 2018

dmikulin committed rL333937: In thin and full LTO + CFI, direct function calls may go through jump table.
In thin and full LTO + CFI, direct function calls may go through jump table
Jun 4 2018, 11:22 AM
dmikulin closed D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.
Jun 4 2018, 11:22 AM

May 31 2018

dmikulin updated the diff for D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

Changed the checks to look for dso_local instread of visibility.

May 31 2018, 1:37 PM

May 30 2018

dmikulin added a comment to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

Can you guys please give it a try with Chromium? Do you get any measurable performance improvements with this change?

May 30 2018, 4:30 PM

May 24 2018

dmikulin added inline comments to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.
May 24 2018, 2:33 PM
dmikulin updated the diff for D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

Changed the code not to short circuit functions that can be overridden at run time and not to expose <func>.cfi names to the runtime.
Added a test point to check for that.

May 24 2018, 2:22 AM

May 21 2018

dmikulin added a comment to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

Oh, this must be with cross-DSO CFI. I see what the problem is. We need to skip symbols not resolved internally from replacing direct calls to calls to <name>.cfi. An extra test case wound't hurt, so if you have it please send it to me.

May 21 2018, 8:47 PM
dmikulin added a comment to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

Based on a quick look at a couple of stack traces it looks like some direct
calls now call symbols that are no longer overriden by their replacements
in chromium, e.g. calls to realloc() from a DSO previously resolved to the
tcmalloc implementation in chromium instead of realloc.cfi(), the locally
linked tcmalloc implementation in that library.

May 21 2018, 8:07 PM
dmikulin added a comment to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

Sorry, I'm traveling this week, didn't see this until now. I didn't get any bot failure notifications after the commit, so I assumed it was good. Do you have a test case I can use to reproduce it?

May 21 2018, 5:01 PM

May 17 2018

dmikulin committed rL332610: In thin and full LTO + CFI, direct function calls may go through jump table.
In thin and full LTO + CFI, direct function calls may go through jump table
May 17 2018, 7:33 AM
dmikulin closed D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.
May 17 2018, 7:33 AM

May 15 2018

dmikulin added inline comments to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.
May 15 2018, 3:42 PM

May 10 2018

dmikulin updated the diff for D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

Fixed a few missed cases where direct calls were routed through jump tables.
Added more test points for thin and full LTO.

May 10 2018, 5:24 PM

May 8 2018

dmikulin added inline comments to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.
May 8 2018, 5:53 PM

May 7 2018

dmikulin committed rL331680: Remove explicit setting of the CFI jumptable section name, it does not appear.
Remove explicit setting of the CFI jumptable section name, it does not appear
May 7 2018, 2:33 PM
dmikulin closed D46537: Remove explicit setting of the CFI jumptable section name..
May 7 2018, 2:33 PM
dmikulin created D46537: Remove explicit setting of the CFI jumptable section name..
May 7 2018, 11:04 AM
dmikulin updated the diff for D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

Added a test case.
Skip direct calls in addition to blockaddr uses when dealing with CFI.
Don't set linkage to internal for .cfi symbols as they may be needed by direct calls.

May 7 2018, 10:51 AM

May 4 2018

dmikulin added a comment to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

ToT LTO and ThinLTO builds both work fine for me. Can you post the error you get? I'm not sure how ToT would produce a direct call to a function with a jumptable entry.

May 4 2018, 1:54 PM

May 3 2018

dmikulin added a comment to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

$ clang++ -fuse-ld=lld -flto=thin -fsanitize=cfi-icall -fvisibility=hidden -o ex ex1.cpp ex2.cpp
lld: error: undefined symbol: void X<double>() (.cfi)

referenced by ex1.cpp

lto.tmp:(main)
May 3 2018, 1:24 PM
dmikulin added a comment to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

No, that statement is doing the reverse of what you're doing. It's replacing directs calls to a function to calls to the jumptable entry. (F/FDecl have different meanings in the 2 other cases than how you're using them.)

Sounds like we need to skip direct calls in replaceUsesExceptBlockAddr(). Make it replaceUsesExceptBlockAddrOrDirectCalls()

May 3 2018, 11:31 AM
dmikulin added a comment to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

No, that statement is doing the reverse of what you're doing. It's replacing directs calls to a function to calls to the jumptable entry. (F/FDecl have different meanings in the 2 other cases than how you're using them.)

May 3 2018, 8:49 AM

May 2 2018

dmikulin added a comment to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

Also adding a FDecl->replaceDirectCalls(F) in the else block after line 1016 seems like it would also correctly rewrite direct calls for the other two cases (external functions, and local functions declared in the current TU.)

May 2 2018, 10:25 AM

May 1 2018

dmikulin added a comment to D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.

Yeah, need a test case.

May 1 2018, 7:57 PM
dmikulin created D46327: Move LowerTypeTestsPass after inlining in ThinLTO pipeline.
May 1 2018, 11:39 AM
dmikulin created D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries.
May 1 2018, 11:34 AM

Mar 5 2018

dmikulin committed rL326737: On Windows we need to be able to process response files with Windows-style.
On Windows we need to be able to process response files with Windows-style
Mar 5 2018, 11:37 AM
dmikulin closed D43988: Fix processing of path names in response files on Windows.
Mar 5 2018, 11:37 AM
dmikulin committed rL326734: On Windows expansion of regex file name patterns is the responsibility of each.
On Windows expansion of regex file name patterns is the responsibility of each
Mar 5 2018, 10:57 AM
dmikulin closed D43987: Fix ar command line expansion on Windows..
Mar 5 2018, 10:57 AM
dmikulin updated the diff for D43988: Fix processing of path names in response files on Windows.

Addressed review comments.

Mar 5 2018, 9:34 AM

Mar 2 2018

dmikulin committed rL326636: Implementation of MRI "delete" command..
Implementation of MRI "delete" command.
Mar 2 2018, 3:26 PM
dmikulin closed D43989: MRI delete command.
Mar 2 2018, 3:26 PM
dmikulin updated the diff for D43989: MRI delete command.

Changed deleteMember() to erase_if()

Mar 2 2018, 1:56 PM
dmikulin updated the diff for D43989: MRI delete command.

Changed the implementation to avoid writing the archive twice.

Mar 2 2018, 12:55 PM
dmikulin updated the diff for D43989: MRI delete command.

llvm-ar supports only a subset of MRI commands, that's why 'delete' as implemented here only works for a specific use case: deleting members added with 'addlib' or 'addmod' commands during archive creation.

Mar 2 2018, 10:01 AM

Mar 1 2018

dmikulin created D43989: MRI delete command.
Mar 1 2018, 5:06 PM
dmikulin created D43988: Fix processing of path names in response files on Windows.
Mar 1 2018, 4:52 PM
dmikulin added a reviewer for D43987: Fix ar command line expansion on Windows.: Bigcheese.
Mar 1 2018, 4:48 PM
dmikulin created D43987: Fix ar command line expansion on Windows..
Mar 1 2018, 4:48 PM
dmikulin created D43986: Allow MRI scripts as file input on cmd line.
Mar 1 2018, 4:44 PM

Feb 8 2018

dmikulin committed rL324670: Minor tweak to test case..
Minor tweak to test case.
Feb 8 2018, 3:12 PM
dmikulin committed rL324658: [ThinLTO] Skip BlockAddresses while replacing uses in function import..
[ThinLTO] Skip BlockAddresses while replacing uses in function import.
Feb 8 2018, 2:17 PM
dmikulin closed D43027: [ThinLTO] Skip BlockAddresses while replacing uses in function import.
Feb 8 2018, 2:16 PM
dmikulin updated the diff for D43027: [ThinLTO] Skip BlockAddresses while replacing uses in function import.

Fixed the test.

Feb 8 2018, 1:10 PM
dmikulin added inline comments to D43027: [ThinLTO] Skip BlockAddresses while replacing uses in function import.
Feb 8 2018, 1:10 PM
dmikulin added inline comments to D43027: [ThinLTO] Skip BlockAddresses while replacing uses in function import.
Feb 8 2018, 12:54 PM
dmikulin updated the diff for D43027: [ThinLTO] Skip BlockAddresses while replacing uses in function import.

Re-wrote the test.

Feb 8 2018, 11:40 AM

Feb 7 2018

dmikulin committed rL324559: Symbols defined in linker scripts should not have dso_local flag set in LTO….
Symbols defined in linker scripts should not have dso_local flag set in LTO…
Feb 7 2018, 8:27 PM
dmikulin committed rLLD324559: Symbols defined in linker scripts should not have dso_local flag set in LTO….
Symbols defined in linker scripts should not have dso_local flag set in LTO…
Feb 7 2018, 8:27 PM
dmikulin closed D43051: Symbols defined in linker scripts should not have dso_local flag set in LTO resolutions.
Feb 7 2018, 8:27 PM
dmikulin updated the diff for D43051: Symbols defined in linker scripts should not have dso_local flag set in LTO resolutions.

Changed the test to create linker script with "echo" instead.

Feb 7 2018, 5:59 PM
dmikulin created D43051: Symbols defined in linker scripts should not have dso_local flag set in LTO resolutions.
Feb 7 2018, 4:27 PM
dmikulin added reviewers for D43027: [ThinLTO] Skip BlockAddresses while replacing uses in function import: davide, tejohnson.
Feb 7 2018, 10:12 AM
dmikulin created D43027: [ThinLTO] Skip BlockAddresses while replacing uses in function import.
Feb 7 2018, 10:02 AM

Feb 6 2018

dmikulin committed rLLD324435: Don't set dso_local flag in LTO resolutions for absolute symbols defined in ELF.
Don't set dso_local flag in LTO resolutions for absolute symbols defined in ELF
Feb 6 2018, 4:52 PM
dmikulin committed rL324435: Don't set dso_local flag in LTO resolutions for absolute symbols defined in ELF.
Don't set dso_local flag in LTO resolutions for absolute symbols defined in ELF
Feb 6 2018, 4:51 PM
dmikulin closed D42977: Don't set dso_local flag in LTO resolutions for absolute symbols defined in ELF objects.
Feb 6 2018, 4:51 PM
dmikulin updated the diff for D42977: Don't set dso_local flag in LTO resolutions for absolute symbols defined in ELF objects.

Minor tweaks.

Feb 6 2018, 3:48 PM
dmikulin updated the diff for D42977: Don't set dso_local flag in LTO resolutions for absolute symbols defined in ELF objects.

Addressed review comments

Feb 6 2018, 3:04 PM
dmikulin created D42977: Don't set dso_local flag in LTO resolutions for absolute symbols defined in ELF objects.
Feb 6 2018, 11:02 AM

Nov 16 2017

dmikulin committed rL318477: Current implementation of Value::replaceUsesExceptBlockAddr() uses UseList.
Current implementation of Value::replaceUsesExceptBlockAddr() uses UseList
Nov 16 2017, 4:31 PM
dmikulin closed D39864: Fix for CFI type tests lowering assert. by committing rL318477: Current implementation of Value::replaceUsesExceptBlockAddr() uses UseList.
Nov 16 2017, 4:31 PM
dmikulin added a comment to D39864: Fix for CFI type tests lowering assert. .

And the set type is changed to SmallSetVector.

Nov 16 2017, 2:37 PM
dmikulin added inline comments to D39864: Fix for CFI type tests lowering assert. .
Nov 16 2017, 1:02 PM
dmikulin updated the diff for D39864: Fix for CFI type tests lowering assert. .

Changed the fix to save unique Constant users and process them separately outside of the UseList iterator.

Nov 16 2017, 12:34 PM
dmikulin added a comment to D39864: Fix for CFI type tests lowering assert. .

Nag... Can you guys please review this?

Nov 16 2017, 8:47 AM

Nov 10 2017

dmikulin added inline comments to D39864: Fix for CFI type tests lowering assert. .
Nov 10 2017, 12:52 PM

Nov 9 2017

dmikulin created D39864: Fix for CFI type tests lowering assert. .
Nov 9 2017, 12:45 PM

Sep 27 2017

dmikulin committed rL314365: ASan allocates a global data initialization array at the tail end of each.
ASan allocates a global data initialization array at the tail end of each
Sep 27 2017, 4:33 PM
dmikulin closed D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section by committing rL314365: ASan allocates a global data initialization array at the tail end of each.
Sep 27 2017, 4:33 PM

Sep 26 2017

dmikulin added a comment to D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.

Can someone please click "Approve"? ;)

Sep 26 2017, 3:30 PM

Sep 25 2017

dmikulin updated the diff for D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.

Updated diff.

Sep 25 2017, 5:02 AM

Sep 22 2017

dmikulin added a comment to D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.

The compiler change from the previous patch is out, so there is no flag.

Sep 22 2017, 9:50 AM

Sep 21 2017

dmikulin updated the diff for D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.

Updated the patch. Do it all in runtime.

Sep 21 2017, 7:55 PM
dmikulin added a comment to D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.

I guess we can do it purely in the runtime code. Initially I thought of allocating another metadata record in the compiler to direct poisoning. But the following does the trick:

Sep 21 2017, 5:05 PM
dmikulin added a comment to D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.

Poisoning the metadata is doable too but I thought it was cleaner to separate user data from metadata.

Sep 21 2017, 4:13 PM
dmikulin added a comment to D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.

This is how .data section is laid out today under ASan:
sym1->RZ1->sym2->RZ2-> ... ->symN->RZN->ASan Metadata Array
The metadata array provides instructions to the runtime on how to poison memory around ASan-ified globals. At runtime red zones are poisoned, but ASan metadata is not. This creates an area at the tail of each compunit data section, which is not poisoned. When sections are concatenated by the linker, it creates unpoisoned holes which should not be accessible to the user. Let's say you have 2 data sections, .data1 followed by .data2, and .data2 has a symbol X at offset 0. Attempts to access memory at &X-1 should be flagged by ASan, but with the current layout they will not be because &X-1 will point into the tail end of the .data1 section where unpoisoned ASan metadata is allocated. Effectively, every symbol at offset 0 in a .data section in each compunit will have no left red zone.
By moving the metadata into a separate section we reduce the number of symbol with no left red zone. All ASan metadata from all objects will be merged by the linker into a single section. The combined .data section will only contain ASan-ified globals.
Hope it clarifies what I'm trying to do with this patch.

Sep 21 2017, 12:06 PM

Sep 20 2017

dmikulin added a comment to D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.

If you look closely, AllGlobals is not holding Asan-ified globals, it's holding the array of MetadataInitializers. Maybe the previous choice of the variable name wasn't descriptive, I can change it.

Sep 20 2017, 4:44 PM

Sep 19 2017

dmikulin updated the diff for D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.

Made the change optional.

Sep 19 2017, 5:41 PM
dmikulin added a comment to D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.

-fdata-sections places each data symbol into a separate section and all ASan metadata goes into its own special section, like "asan_globals" on Linux.

Sep 19 2017, 3:54 PM
dmikulin added a comment to D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.
In D38056#875680, @kcc wrote:

Please put this under a flag, off by default. (something like -mllvm -asan-globals-separate-section=1)
This is an obvious fix with totally non-obvious consequences, we don't want to find these consequences in the hard way.

Sep 19 2017, 3:22 PM
dmikulin created D38056: Partial fix for bug 34607: ASan misses global underflow in first symbol of data section.
Sep 19 2017, 2:55 PM

Sep 11 2017

dmikulin added a comment to D37718: [ELF] Handle references to garbage collected common symbols.

Forgot about debug...
Looks reasonable to me.

Sep 11 2017, 6:50 PM
dmikulin added a comment to D37718: [ELF] Handle references to garbage collected common symbols.

Why is there a relocation for "unused" if it's marked not live?
Relocation section '.rela.noalloc' at offset 0x98 contains 1 entries:

Offset          Info           Type           Sym. Value    Sym. Name + Addend

000000000000 000200000001 R_X86_64_64 0000000000000004 unused + 0

Sep 11 2017, 4:16 PM

Sep 8 2017

dmikulin committed rL312796: Currently lld creates a single section to collect all commons. There is no way.
Currently lld creates a single section to collect all commons. There is no way
Sep 8 2017, 9:24 AM
dmikulin closed D37489: Linker script handing of file patterns with COMMON defs by committing rL312796: Currently lld creates a single section to collect all commons. There is no way.
Sep 8 2017, 9:24 AM

Sep 7 2017

dmikulin updated the diff for D37489: Linker script handing of file patterns with COMMON defs.

Addressed latest comments.

Sep 7 2017, 4:59 PM
dmikulin updated the diff for D37489: Linker script handing of file patterns with COMMON defs.

Now without sorting the commons by alignment.

Sep 7 2017, 4:01 PM
dmikulin added a comment to D37489: Linker script handing of file patterns with COMMON defs.

Are there Fortran FEs to LLVM? Might still be fairly common there.
If it doesn't cost much, keeping the sorting doesn't hurt, may save us a few alignment holes. Most of the cost is from section per common, not from sorting.

Sep 7 2017, 3:14 PM
dmikulin updated subscribers of D37489: Linker script handing of file patterns with COMMON defs.
Sep 7 2017, 1:45 PM

Sep 6 2017

dmikulin updated the diff for D37489: Linker script handing of file patterns with COMMON defs.

Changed the implementation to allocate a common section for each common symbol.
Per Rui's comment, removed a check for common sections in LinkerScript::discard() to allow discarding them.

Sep 6 2017, 2:07 PM

Sep 5 2017

dmikulin added a comment to D37489: Linker script handing of file patterns with COMMON defs.

I think debug link will mask processing of commons even more, as the number of commons is more or less the same, but link time is dominated by processing debug info.
The synthetic test case confirms the inefficiency of allocating a section per common. The question is: can we tolerate the inefficiency?

Sep 5 2017, 6:46 PM