diff mbox series

[v2,2/2] ftrace: recordmcount: Handle sections with no non-weak symbols

Message ID 126aca34935cf1c7168e17970c706e36577094e7.1651166001.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive)
State Handled Elsewhere
Headers show
Series ftrace/recordmcount: Handle object files without section symbols | expand

Commit Message

Naveen N. Rao April 28, 2022, 5:19 p.m. UTC
Kernel builds on powerpc are failing with the below error [1]:
      CC      kernel/kexec_file.o
    Cannot find symbol for section 9: .text.unlikely.
    kernel/kexec_file.o: failed

Since commit d1bcae833b32f1 ("ELF: Don't generate unused section
symbols") [2], binutils started dropping section symbols that it thought
were unused.  This isn't an issue in general, but with kexec_file.c, gcc
is placing kexec_arch_apply_relocations[_add] into a separate
.text.unlikely section and the section symbol ".text.unlikely" is being
dropped. Due to this, recordmcount is unable to find a non-weak symbol
in .text.unlikely to generate a relocation record against.

Handle this by falling back to a weak symbol, similar to what objtool
does in commit 44f6a7c0755d8d ("objtool: Fix seg fault with Clang
non-section symbols"). This approach however can result in duplicate
and/or invalid addresses in the final vmlinux mcount location table.

As an example, with this commit, relocation records for __mcount_loc for
kexec_file.o now include two entries with the weak functions
arch_kexec_apply_relocations() and arch_kexec_apply_relocation_add() as
the relocation bases:

  ...
  0000000000000080 R_PPC64_ADDR64    .text+0x0000000000001d34
  0000000000000088 R_PPC64_ADDR64    .text+0x0000000000001fec
  0000000000000090 R_PPC64_ADDR64    arch_kexec_apply_relocations_add+0x000000000000000c
  0000000000000098 R_PPC64_ADDR64    arch_kexec_apply_relocations+0x000000000000000c

Powerpc does not override these functions today, so these get converted
to correct offsets in the mcount location table in vmlinux.

If one or both of these weak functions are overridden in future, in the
final vmlinux mcount table, references to these will change over to the
non-weak variant which has its own mcount location entry. As such, there
will now be two entries for these functions.

On ppc32, mcount location is always the third instruction in a function.
On ppc64 with elf abi v2 (ppc64le), mcount location depends on whether
the function has a global entry (fourth instruction) or not (second
instruction), but this is expected to be the same across weak/non-weak
implementations of a function. As such, in both these scenarios, as well
as with other architectures where mcount location is at the same offset
into a function, the two mcount entries will point to the same address.
Ftrace skips the duplicate entries due to a previous commit.

But, with ppc64 elf abi v1 which only supports the old -pg flag, mcount
location can differ between the weak and non-weak variants of a
function. In such scenarios, one of the two mcount entries will be
invalid. Such architectures need to validate mcount locations by
ensuring that the instruction(s) at those locations are as expected. On
powerpc, this can be a simple check to ensure that the instruction is a
'bl'. This check can be further tightened as necessary.

Introduce a config option HAVE_MCOUNT_LOC_VALIDATION that architectures
can select to indicate support for validating the mcount locations
during ftrace initialization. Add a flag (-a) to recordmcount which can
then be passed to allow recordmcount to emit relocation records using
weak symbols as the base.

[1] https://github.com/linuxppc/issues/issues/388
[2] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 Makefile                           |  4 ++
 arch/powerpc/Kconfig               |  1 +
 arch/powerpc/include/asm/ftrace.h  |  8 +--
 arch/powerpc/kernel/trace/ftrace.c | 11 ++++
 kernel/trace/Kconfig               |  6 ++
 scripts/Makefile.build             |  3 +
 scripts/recordmcount.c             |  6 +-
 scripts/recordmcount.h             | 94 ++++++++++++++++++++++++++----
 8 files changed, 113 insertions(+), 20 deletions(-)

Comments

Steven Rostedt April 28, 2022, 10:42 p.m. UTC | #1
On Thu, 28 Apr 2022 22:49:52 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> But, with ppc64 elf abi v1 which only supports the old -pg flag, mcount
> location can differ between the weak and non-weak variants of a
> function. In such scenarios, one of the two mcount entries will be
> invalid. Such architectures need to validate mcount locations by
> ensuring that the instruction(s) at those locations are as expected. On
> powerpc, this can be a simple check to ensure that the instruction is a
> 'bl'. This check can be further tightened as necessary.

I was thinking about this more, and I was thinking that we could create
another section; Perhaps __mcount_loc_weak. And place these in that
section. That way, we could check if these symbols to see if there's
already a symbol for it, and if there is, then drop it.

-- Steve
Naveen N. Rao April 29, 2022, 5:39 p.m. UTC | #2
Steven Rostedt wrote:
> On Thu, 28 Apr 2022 22:49:52 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
>> But, with ppc64 elf abi v1 which only supports the old -pg flag, mcount
>> location can differ between the weak and non-weak variants of a
>> function. In such scenarios, one of the two mcount entries will be
>> invalid. Such architectures need to validate mcount locations by
>> ensuring that the instruction(s) at those locations are as expected. On
>> powerpc, this can be a simple check to ensure that the instruction is a
>> 'bl'. This check can be further tightened as necessary.
> 
> I was thinking about this more, and I was thinking that we could create
> another section; Perhaps __mcount_loc_weak. And place these in that
> section. That way, we could check if these symbols to see if there's
> already a symbol for it, and if there is, then drop it.

If I'm understanding your suggestion right:
- we now create a new section in each object file: __mcount_loc_weak, 
  and capture such relocations using weak symbols there.
- we then ask the linker to put these separately between, say, 
  __start_mcount_loc_weak and __stop_mcount_loc_weak
- on ftrace init, we go through entries in this range, but discard those 
  that belong to functions that also have an entry between 
  __start_mcount_loc and __stop_mcount loc.

The primary issue I see here is that the mcount locations within the new 
weak section will end up being offsets from a different function in 
vmlinux, since the linker does not create a symbol for the weak 
functions that were over-ridden.

As an example, in the issue described in this patch set, if powerpc 
starts over-riding kexec_arch_apply_relocations(), then the current weak 
implementation in kexec_file.o gets carried over to the final vmlinux, 
but the instructions will instead appear under the previous function in 
kexec_file.o: crash_prepare_elf64_headers(). This function may or may 
not be traced to begin with, so we won't be able to figure out if this 
is valid or not.


- Naveen
Steven Rostedt April 29, 2022, 5:59 p.m. UTC | #3
On Fri, 29 Apr 2022 23:09:19 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> If I'm understanding your suggestion right:
> - we now create a new section in each object file: __mcount_loc_weak, 
>   and capture such relocations using weak symbols there.

Yes, but it would be putting the same information it puts into __mcount_loc
but also add it here too. That is, it will duplicate the data.

> - we then ask the linker to put these separately between, say, 
>   __start_mcount_loc_weak and __stop_mcount_loc_weak

Yes, but it will also go in the location between __start_mcount_loc and
__stop_mcount_loc.

> - on ftrace init, we go through entries in this range, but discard those 
>   that belong to functions that also have an entry between 
>   __start_mcount_loc and __stop_mcount loc.

But we should be able to know if it was overridden or not, by seeing if
there's another function that was called. Or at least, we can validate them
to make sure that they are correct.

> 
> The primary issue I see here is that the mcount locations within the new 
> weak section will end up being offsets from a different function in 
> vmlinux, since the linker does not create a symbol for the weak 
> functions that were over-ridden.

The point of this section is to know which functions in __mcount_loc may
have been overridden, as they would be found in the __mcount_loc_weak
section. And then we can do something "special" to them.

> 
> As an example, in the issue described in this patch set, if powerpc 
> starts over-riding kexec_arch_apply_relocations(), then the current weak 
> implementation in kexec_file.o gets carried over to the final vmlinux, 
> but the instructions will instead appear under the previous function in 
> kexec_file.o: crash_prepare_elf64_headers(). This function may or may 
> not be traced to begin with, so we won't be able to figure out if this 
> is valid or not.

If it was overridden, then there would be two entries for function that
overrides the weak function in the __mcount_loc section, right? One for the
new function, and one that was overridden. Of course this could be more
complex if the new function had been marked notrace.

I was thinking of doing this just so that we know what functions are weak
and perhaps need extra processing.

Another issue with weak functions and ftrace just came up here:

  https://lore.kernel.org/all/20220428095803.66c17c32@gandalf.local.home/


-- Steve
Naveen N. Rao April 29, 2022, 7:33 p.m. UTC | #4
Steven Rostedt wrote:
> On Fri, 29 Apr 2022 23:09:19 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
>> If I'm understanding your suggestion right:
>> - we now create a new section in each object file: __mcount_loc_weak, 
>>   and capture such relocations using weak symbols there.
> 
> Yes, but it would be putting the same information it puts into __mcount_loc
> but also add it here too. That is, it will duplicate the data.
> 
>> - we then ask the linker to put these separately between, say, 
>>   __start_mcount_loc_weak and __stop_mcount_loc_weak
> 
> Yes, but it will also go in the location between __start_mcount_loc and
> __stop_mcount_loc.
> 
>> - on ftrace init, we go through entries in this range, but discard those 
>>   that belong to functions that also have an entry between 
>>   __start_mcount_loc and __stop_mcount loc.
> 
> But we should be able to know if it was overridden or not, by seeing if
> there's another function that was called. Or at least, we can validate them
> to make sure that they are correct.
> 
>> 
>> The primary issue I see here is that the mcount locations within the new 
>> weak section will end up being offsets from a different function in 
>> vmlinux, since the linker does not create a symbol for the weak 
>> functions that were over-ridden.
> 
> The point of this section is to know which functions in __mcount_loc may
> have been overridden, as they would be found in the __mcount_loc_weak
> section. And then we can do something "special" to them.

I'm not sure I follow that. How are you intending to figure out which 
functions were overridden by looking at entries in the __mcount_loc_weak 
section?

In the final vmlinux image, we only get offsets into .text for all 
mcount locations, but no symbol information. The only hint is the fact 
that a single kallsym symbol has multiple mcount locations within it. 
Even then, the symbol with duplicate mcount entries won't be the 
function that was overridden.

We could do a kallsyms_lookup() on each entry and consult the 
__mcount_loc_weak section to identify duplicates, but that looks to be 
very expensive.

Did you have a different approach in mind?

> 
>> 
>> As an example, in the issue described in this patch set, if powerpc 
>> starts over-riding kexec_arch_apply_relocations(), then the current weak 
>> implementation in kexec_file.o gets carried over to the final vmlinux, 
>> but the instructions will instead appear under the previous function in 
>> kexec_file.o: crash_prepare_elf64_headers(). This function may or may 
>> not be traced to begin with, so we won't be able to figure out if this 
>> is valid or not.
> 
> If it was overridden, then there would be two entries for function that
> overrides the weak function in the __mcount_loc section, right? One for the
> new function, and one that was overridden.

In the final vmlinux, we will have two entries: one pointing at the 
correct function, while the other will point to some other function 
name. So, at least from kallsym perspective, duplicate mcount entries 
won't be for the function that was overridden, but some arbitrary 
function that came before the weak function in the object file.

> Of course this could be more
> complex if the new function had been marked notrace.
> 
> I was thinking of doing this just so that we know what functions are weak
> and perhaps need extra processing.
> 
> Another issue with weak functions and ftrace just came up here:
> 
>   https://lore.kernel.org/all/20220428095803.66c17c32@gandalf.local.home/

I noticed this just yesterday:

  # cat available_filter_functions | sort | uniq -d | wc -l
  430

I'm fairly certain that some of those are due to weak functions -- I 
just wasn't sure if all of those were.

I suppose this will now also be a problem with ftrace_location(), given 
that it was recently changed to look at an entire function for mcount 
locations?


Thanks,
Naveen
Steven Rostedt April 29, 2022, 7:47 p.m. UTC | #5
On Sat, 30 Apr 2022 01:03:01 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> > The point of this section is to know which functions in __mcount_loc may
> > have been overridden, as they would be found in the __mcount_loc_weak
> > section. And then we can do something "special" to them.  
> 
> I'm not sure I follow that. How are you intending to figure out which 
> functions were overridden by looking at entries in the __mcount_loc_weak 
> section?

If there's duplicates (or something strange with the offset) then we could
delete the one that has the match in the weak section.

> 
> In the final vmlinux image, we only get offsets into .text for all 
> mcount locations, but no symbol information. The only hint is the fact 
> that a single kallsym symbol has multiple mcount locations within it. 
> Even then, the symbol with duplicate mcount entries won't be the 
> function that was overridden.
> 
> We could do a kallsyms_lookup() on each entry and consult the 
> __mcount_loc_weak section to identify duplicates, but that looks to be 
> very expensive.
> 
> Did you have a different approach in mind?

We only need to look at the ones in the weak section. How many that is may
determine if that's feasible or not.

> 
> >   
> >> 
> >> As an example, in the issue described in this patch set, if powerpc 
> >> starts over-riding kexec_arch_apply_relocations(), then the current weak 
> >> implementation in kexec_file.o gets carried over to the final vmlinux, 
> >> but the instructions will instead appear under the previous function in 
> >> kexec_file.o: crash_prepare_elf64_headers(). This function may or may 
> >> not be traced to begin with, so we won't be able to figure out if this 
> >> is valid or not.  
> > 
> > If it was overridden, then there would be two entries for function that
> > overrides the weak function in the __mcount_loc section, right? One for the
> > new function, and one that was overridden.  
> 
> In the final vmlinux, we will have two entries: one pointing at the 
> correct function, while the other will point to some other function 
> name. So, at least from kallsym perspective, duplicate mcount entries 
> won't be for the function that was overridden, but some arbitrary 
> function that came before the weak function in the object file.

Right, and we should be able to find them.

> 
> > Of course this could be more
> > complex if the new function had been marked notrace.
> > 
> > I was thinking of doing this just so that we know what functions are weak
> > and perhaps need extra processing.
> > 
> > Another issue with weak functions and ftrace just came up here:
> > 
> >   https://lore.kernel.org/all/20220428095803.66c17c32@gandalf.local.home/  
> 
> I noticed this just yesterday:
> 
>   # cat available_filter_functions | sort | uniq -d | wc -l
>   430
> 
> I'm fairly certain that some of those are due to weak functions -- I 
> just wasn't sure if all of those were.

Probably :-/

> 
> I suppose this will now also be a problem with ftrace_location(), given 
> that it was recently changed to look at an entire function for mcount 
> locations?
> 

Maybe. I have to focus on other things at the moment, but I'll try to find
some time to look at this deeper.

-- Steve
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 29e273d3f8ccbf..b2a9fdb49815fb 100644
--- a/Makefile
+++ b/Makefile
@@ -858,6 +858,10 @@  ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
     BUILD_C_RECORDMCOUNT := y
     export BUILD_C_RECORDMCOUNT
   endif
+  ifdef CONFIG_HAVE_MCOUNT_LOC_VALIDATION
+    HAVE_MCOUNT_LOC_VALIDATION := y
+    export HAVE_MCOUNT_LOC_VALIDATION
+  endif
 endif
 ifdef CONFIG_HAVE_FENTRY
   # s390-linux-gnu-gcc did not support -mfentry until gcc-9.
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 174edabb74fa11..acae4085aa6d6b 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -229,6 +229,7 @@  config PPC
 	select HAVE_KRETPROBES
 	select HAVE_LD_DEAD_CODE_DATA_ELIMINATION
 	select HAVE_LIVEPATCH			if HAVE_DYNAMIC_FTRACE_WITH_REGS
+	select HAVE_MCOUNT_LOC_VALIDATION
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_NMI				if PERF_EVENTS || (PPC64 && PPC_BOOK3S)
 	select HAVE_OPTPROBES
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index d83758acd1c7c3..d8b104ed2fdf38 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -12,13 +12,7 @@ 
 
 #ifndef __ASSEMBLY__
 extern void _mcount(void);
-
-static inline unsigned long ftrace_call_adjust(unsigned long addr)
-{
-       /* relocation of mcount call site is the same as the address */
-       return addr;
-}
-
+unsigned long ftrace_call_adjust(unsigned long addr);
 unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
 				    unsigned long sp);
 
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 4ee04aacf9f13c..976c08cd0573f7 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -858,6 +858,17 @@  void arch_ftrace_update_code(int command)
 	ftrace_modify_all_code(command);
 }
 
+unsigned long ftrace_call_adjust(unsigned long addr)
+{
+	ppc_inst_t op = ppc_inst_read((u32 *)addr);
+
+	if (!is_bl_op(op))
+		return 0;
+
+	/* relocation of mcount call site is the same as the address */
+	return addr;
+}
+
 #ifdef CONFIG_PPC64
 #define PACATOC offsetof(struct paca_struct, kernel_toc)
 
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 2c43e327a619f7..f1704c764fa66a 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -81,6 +81,12 @@  config HAVE_C_RECORDMCOUNT
 	help
 	  C version of recordmcount available?
 
+config HAVE_MCOUNT_LOC_VALIDATION
+	bool
+	help
+	  Arch validates mcount locations and can cope with invalid entries
+	  in the mcount table.
+
 config HAVE_BUILDTIME_MCOUNT_SORT
        bool
        help
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 9717e6f6fb3149..4401103991fd5c 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -199,6 +199,9 @@  ifdef BUILD_C_RECORDMCOUNT
 ifeq ("$(origin RECORDMCOUNT_WARN)", "command line")
   RECORDMCOUNT_FLAGS = -w
 endif
+ifdef HAVE_MCOUNT_LOC_VALIDATION
+  RECORDMCOUNT_FLAGS += -a
+endif
 # Due to recursion, we must skip empty.o.
 # The empty.o file is created in the make process in order to determine
 # the target endianness and word size. It is made before all other C
diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index cce12e1971d853..30308109446e86 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -50,6 +50,7 @@  static char gpfx;	/* prefix for global symbol name (sometimes '_') */
 static struct stat sb;	/* Remember .st_size, etc. */
 static const char *altmcount;	/* alternate mcount symbol name */
 static int warn_on_notrace_sect; /* warn when section has mcount not being recorded */
+static int allow_weak_sym_rel; /* allow relocations using weak symbols */
 static void *file_map;	/* pointer of the mapped file */
 static void *file_end;	/* pointer to the end of the mapped file */
 static int file_updated; /* flag to state file was changed */
@@ -631,8 +632,11 @@  int main(int argc, char *argv[])
 	int c;
 	int i;
 
-	while ((c = getopt(argc, argv, "w")) >= 0) {
+	while ((c = getopt(argc, argv, "aw")) >= 0) {
 		switch (c) {
+		case 'a':
+			allow_weak_sym_rel = 1;
+			break;
 		case 'w':
 			warn_on_notrace_sect = 1;
 			break;
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 1e9baa5c4fc6ef..c3817b3679e601 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -25,6 +25,7 @@ 
 #undef sift_rel_mcount
 #undef nop_mcount
 #undef find_secsym_ndx
+#undef find_sym_ndx
 #undef __has_rel_mcount
 #undef has_rel_mcount
 #undef tot_relsize
@@ -60,6 +61,7 @@ 
 # define sift_rel_mcount	sift64_rel_mcount
 # define nop_mcount		nop_mcount_64
 # define find_secsym_ndx	find64_secsym_ndx
+# define find_sym_ndx		find64_sym_ndx
 # define __has_rel_mcount	__has64_rel_mcount
 # define has_rel_mcount		has64_rel_mcount
 # define tot_relsize		tot64_relsize
@@ -98,6 +100,7 @@ 
 # define sift_rel_mcount	sift32_rel_mcount
 # define nop_mcount		nop_mcount_32
 # define find_secsym_ndx	find32_secsym_ndx
+# define find_sym_ndx		find32_sym_ndx
 # define __has_rel_mcount	__has32_rel_mcount
 # define has_rel_mcount		has32_rel_mcount
 # define tot_relsize		tot32_relsize
@@ -392,6 +395,51 @@  static void get_sym_str_and_relp(Elf_Shdr const *const relhdr,
 	*relp = rel0;
 }
 
+/*
+ * Find a symbol in the given section containing the instruction offset passed
+ * in r_offset, to be used in generating the relocation record for the mcount
+ * location. This is used if there were no local/global symbols in the given
+ * section to be used as the base. Weak symbols are ok, and are expected to
+ * result in duplicate mcount locations which get dropped by ftrace.
+ */
+static int find_sym_ndx(unsigned const txtndx,
+			 char const *const txtname,
+			 uint_t *const recvalp,
+			 unsigned int *sym_index,
+			 Elf_Shdr const *const symhdr,
+			 Elf32_Word const *symtab,
+			 Elf32_Word const *symtab_shndx,
+			 unsigned const r_offset,
+			 Elf_Ehdr const *const ehdr)
+{
+	Elf_Sym const *const sym0 = (Elf_Sym const *)(_w(symhdr->sh_offset)
+		+ (void *)ehdr);
+	unsigned const nsym = _w(symhdr->sh_size) / _w(symhdr->sh_entsize);
+	Elf_Sym const *symp;
+	unsigned t;
+
+	for (symp = sym0, t = nsym; t; --t, ++symp) {
+		if (txtndx == get_symindex(symp, symtab, symtab_shndx)) {
+			/* function symbols on ARM have quirks, avoid them */
+			if (w2(ehdr->e_machine) == EM_ARM &&
+			    ELF_ST_TYPE(symp->st_info) == STT_FUNC)
+				continue;
+
+			if (r_offset >= _w(symp->st_value) &&
+			    r_offset < _w(symp->st_value) + _w(symp->st_size)) {
+				*recvalp = _w(symp->st_value);
+				*sym_index = symp - sym0;
+				return 0;
+			}
+		}
+	}
+
+	fprintf(stderr, "Cannot find symbol containing offset %u for section %u: %s.\n",
+		r_offset, txtndx, txtname);
+
+	return -1;
+}
+
 /*
  * Look at the relocations in order to find the calls to mcount.
  * Accumulate the section offsets that are found, and their relocation info,
@@ -402,9 +450,14 @@  static uint_t *sift_rel_mcount(uint_t *mlocp,
 			       Elf_Rel **const mrelpp,
 			       Elf_Shdr const *const relhdr,
 			       Elf_Ehdr const *const ehdr,
-			       unsigned const recsym,
-			       uint_t const recval,
-			       unsigned const reltype)
+			       unsigned recsym,
+			       uint_t recval,
+			       unsigned const reltype,
+			       unsigned int no_secsym,
+			       char const *const txtname,
+			       Elf_Shdr const *const shdr0,
+			       Elf32_Word const *symtab,
+			       Elf32_Word const *symtab_shndx)
 {
 	uint_t *const mloc0 = mlocp;
 	Elf_Rel *mrelp = *mrelpp;
@@ -415,6 +468,7 @@  static uint_t *sift_rel_mcount(uint_t *mlocp,
 	unsigned const nrel = _w(relhdr->sh_size) / rel_entsize;
 	unsigned mcountsym = 0;
 	unsigned t;
+	uint_t addend;
 
 	get_sym_str_and_relp(relhdr, ehdr, &sym0, &str0, &relp);
 
@@ -424,8 +478,13 @@  static uint_t *sift_rel_mcount(uint_t *mlocp,
 
 		if (mcountsym && mcountsym == Elf_r_sym(relp) &&
 				!is_fake_mcount(relp)) {
-			uint_t const addend =
-				_w(_w(relp->r_offset) - recval + mcount_adjust);
+			if (no_secsym && find_sym_ndx(w(relhdr->sh_info),
+						      txtname, &recval, &recsym,
+						      &shdr0[w(relhdr->sh_link)],
+						      symtab, symtab_shndx,
+						      _w(relp->r_offset), ehdr))
+				return 0;
+			addend = _w(_w(relp->r_offset) - recval + mcount_adjust);
 			mrelp->r_offset = _w(offbase
 				+ ((void *)mlocp - (void *)mloc0));
 			Elf_r_info(mrelp, recsym, reltype);
@@ -544,8 +603,9 @@  static int find_secsym_ndx(unsigned const txtndx,
 			return 0;
 		}
 	}
-	fprintf(stderr, "Cannot find symbol for section %u: %s.\n",
-		txtndx, txtname);
+	if (!allow_weak_sym_rel)
+		fprintf(stderr, "Cannot find symbol for section %u: %s.\n",
+			txtndx, txtname);
 	return -1;
 }
 
@@ -660,22 +720,32 @@  static int do_func(Elf_Ehdr *const ehdr, char const *const fname,
 			goto out; /* Nothing to be done; don't append! */
 		}
 		if (txtname && is_mcounted_section_name(txtname)) {
-			unsigned int recsym;
+			unsigned int recsym = 0, no_secsym = 0;
 			uint_t recval = 0;
 
 			symsec_sh_link = w(relhdr->sh_link);
-			result = find_secsym_ndx(w(relhdr->sh_info), txtname,
+			if (find_secsym_ndx(w(relhdr->sh_info), txtname,
 						&recval, &recsym,
 						&shdr0[symsec_sh_link],
 						symtab, symtab_shndx,
-						ehdr);
-			if (result)
+						ehdr))
+				no_secsym = 1;
+
+			if (no_secsym && !allow_weak_sym_rel) {
+				result = -1;
 				goto out;
+			}
 
 			rel_entsize = _w(relhdr->sh_entsize);
 			mlocp = sift_rel_mcount(mlocp,
 				(void *)mlocp - (void *)mloc0, &mrelp,
-				relhdr, ehdr, recsym, recval, reltype);
+				relhdr, ehdr, recsym, recval, reltype,
+				no_secsym, txtname, shdr0, symtab,
+				symtab_shndx);
+			if (!mlocp) {
+				result = -1;
+				goto out;
+			}
 		} else if (txtname && (warn_on_notrace_sect || make_nop)) {
 			/*
 			 * This section is ignored by ftrace, but still