This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
Re: Simple Objloader patch
- From: John Dallaway <john at dallaway dot org dot uk>
- To: ecos-patches at ecos dot sourceware dot org
- Date: Fri, 09 Oct 2009 15:14:09 +0100
- Subject: Re: Simple Objloader patch
- References: <b437ec690909141015x16581876j8f0f101f9d0eabf6@mail.gmail.com>
Hi Anthony
Anthony Tonizzo wrote:
> Davy Wouters pointed out a memory leak on the objoader
> package last month. The attached patch fixes the problem
> and adds a few more checks on returned values.
Thank you for the patch.
I discovered a couple of CDL issues during testing and I've also cleaned
out the Cygwin tclsh workarounds which are now obsolete and have been
removed elsewhere. Everything is now checked-in. Final patch attached.
John Dallaway
Index: services/objloader/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/services/objloader/current/ChangeLog,v
retrieving revision 1.6
diff -U5 -r1.6 ChangeLog
--- services/objloader/current/ChangeLog 3 Jul 2009 14:49:44 -0000 1.6
+++ services/objloader/current/ChangeLog 9 Oct 2009 13:21:48 -0000
@@ -1,5 +1,18 @@
+2009-10-09 John Dallaway <john@dallaway.org.uk>
+
+ * cdl/objloader.cdl: Eliminate workarounds for file path handling
+ issue in obsolete Cygwin tclsh. Use ACTUAL_CFLAGS for robustness.
+
+2009-09-13 Anthony Tonizzo <atonizzo@gmail.com>
+
+ * src/objloader.c, src/objelf.c, include/objelf.h : Fixed a memory
+ leak where a library section was loaded but memory allocated was not
+ released. This bug was reported by Davy Wouters on the eCos list.
+ Added minor cosmetics and a number of CYG_ASSERT and if() to test the
+ values returnedby cyg_ldr_load_elf_section().
+
2009-07-03 John Dallaway <john@dallaway.org.uk>
* cdl/objloader.cdl, src/objelf.c, src/relocate_ppc.c,
src/relocate_arm.c: Eliminate dependency on CYGPKG_IO_FILEIO when
the filesystem loader is not required.
Index: services/objloader/current/include/objelf.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/services/objloader/current/include/objelf.h,v
retrieving revision 1.5
diff -U5 -r1.5 objelf.h
--- services/objloader/current/include/objelf.h 3 Jul 2009 14:49:44 -0000 1.5
+++ services/objloader/current/include/objelf.h 9 Oct 2009 13:21:49 -0000
@@ -7,11 +7,11 @@
*
* =================================================================
* ####ECOSGPLCOPYRIGHTBEGIN####
* -------------------------------------------
* This file is part of eCos, the Embedded Configurable Operating System.
- * Copyright (C) 2005, 2008 Free Software Foundation, Inc.
+ * Copyright (C) 2005, 2008, 2009 Free Software Foundation, Inc.
*
* eCos is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License as published by the Free
* Software Foundation; either version 2 or (at your option) any later
* version.
@@ -96,13 +96,13 @@
//==============================================================================
// Debug functions.
#if CYGPKG_SERVICES_OBJLOADER_DEBUG_LEVEL > 0
-void cyg_ldr_print_section_data(PELF_OBJECT);
-void cyg_ldr_print_symbol_names(PELF_OBJECT);
-void cyg_ldr_print_rel_names(PELF_OBJECT);
+void cyg_ldr_print_section_data(PELF_OBJECT);
+void cyg_ldr_print_symbol_names(PELF_OBJECT);
+cyg_int32 cyg_ldr_print_rel_names(PELF_OBJECT);
#endif
//==============================================================================
// Internal functions
Index: services/objloader/current/src/objelf.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/services/objloader/current/src/objelf.c,v
retrieving revision 1.5
diff -U5 -r1.5 objelf.c
--- services/objloader/current/src/objelf.c 3 Jul 2009 14:49:45 -0000 1.5
+++ services/objloader/current/src/objelf.c 9 Oct 2009 13:21:49 -0000
@@ -49,10 +49,11 @@
*
* =================================================================
*/
#include <cyg/infra/diag.h> // For diagnostic printing.
+#include <cyg/infra/cyg_ass.h>
#include <cyg/hal/hal_tables.h>
#include <stdio.h>
#include <pkgconf/objloader.h>
#include <cyg/objloader/elf.h>
@@ -114,11 +115,11 @@
p_symtab[i].st_shndx,
p_strtab + p_symtab[i].st_name);
diag_printf("\n");
}
-void
+cyg_int32
cyg_ldr_print_rel_names(PELF_OBJECT p)
{
int i, j, r_entries, sym_index;
Elf32_Sym *p_symtab = (Elf32_Sym*)p->sections[p->hdrndx_symtab];
char *p_strtab = (char*)p->sections[p->hdrndx_strtab];
@@ -133,20 +134,24 @@
for (i = 1; i < p->p_elfhdr->e_shnum; i++)
{
if ((p->p_sechdr[i].sh_type == SHT_REL) ||
(p->p_sechdr[i].sh_type == SHT_RELA))
{
- // Calculate the total number of entries in the .rela section.
+ // Calculate the total number of entries in the .rela/.rel section.
r_entries = p->p_sechdr[i].sh_size / p->p_sechdr[i].sh_entsize;
diag_printf("\n\nSymbols at: %s\n\n",
p_shstrtab + p->p_sechdr[i].sh_name);
#if ELF_ARCH_RELTYPE == Elf_Rela
- p_rela = (Elf32_Rela*)cyg_ldr_load_elf_section(p, i);
+ p_rela = (Elf32_Rela *)cyg_ldr_load_elf_section(p, i);
+ if (p_rela == 0)
+ return -1;
printf("Offset Info Name [+ Addend]\n");
#else
- p_rel = (Elf32_Rel*)cyg_ldr_load_elf_section(p, i);
+ p_rel = (Elf32_Rel *)cyg_ldr_load_elf_section(p, i);
+ if (p_rel == 0)
+ return -1;
printf("Offset Info Name\n");
#endif
for (j = 0; j < r_entries; j++)
{
@@ -286,21 +291,25 @@
cyg_int32
cyg_ldr_relocate_section(PELF_OBJECT p, cyg_uint32 r_shndx)
{
int i, rc;
#if ELF_ARCH_RELTYPE == Elf_Rela
- Elf32_Rela* p_rela = (Elf32_Rela*)cyg_ldr_load_elf_section(p, r_shndx);
+ Elf32_Rela *p_rela = (Elf32_Rela *)cyg_ldr_load_elf_section(p, r_shndx);
+ if (p_rela == 0)
+ return -1;
#else
- Elf32_Rel* p_rel = (Elf32_Rel*)cyg_ldr_load_elf_section(p, r_shndx);
+ Elf32_Rel *p_rel = (Elf32_Rel *)cyg_ldr_load_elf_section(p, r_shndx);
+ if (p_rel == 0)
+ return -1;
#endif
#if CYGPKG_SERVICES_OBJLOADER_DEBUG_LEVEL > 0
- Elf32_Sym *p_symtab = (Elf32_Sym*)cyg_ldr_section_address(p,
+ Elf32_Sym *p_symtab = (Elf32_Sym *)cyg_ldr_section_address(p,
p->hdrndx_symtab);
- char *p_strtab = (char*)cyg_ldr_section_address(p, p->hdrndx_strtab);
- char *p_shstrtab = (char*)cyg_ldr_section_address(p,
- p->p_elfhdr->e_shstrndx);
+ char *p_strtab = (char *)cyg_ldr_section_address(p, p->hdrndx_strtab);
+ char *p_shstrtab = (char *)cyg_ldr_section_address(p,
+ p->p_elfhdr->e_shstrndx);
#endif
// Now we can get the address of the contents of the section to modify.
cyg_uint32 r_target_shndx = p->p_sechdr[r_shndx].sh_info;
cyg_uint32 r_target_addr = (cyg_uint32)cyg_ldr_section_address(p,
@@ -309,11 +318,11 @@
#if CYGPKG_SERVICES_OBJLOADER_DEBUG_LEVEL > 0
diag_printf("Relocating section \"%s\"\n",
p_shstrtab + p->p_sechdr[r_target_shndx].sh_name);
diag_printf("----------------------------------------\n");
#if CYGPKG_SERVICES_OBJLOADER_DEBUG_LEVEL > 1
- diag_printf(" Ndx Type Offset Name\"\n");
+ diag_printf(" Ndx Type Offset Name\n");
#endif
#endif
// Perform relocatation for each of the members of this table.
cyg_uint32 r_entries = p->p_sechdr[r_shndx].sh_size /
Index: services/objloader/current/src/objloader.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/services/objloader/current/src/objloader.c,v
retrieving revision 1.5
diff -U5 -r1.5 objloader.c
--- services/objloader/current/src/objloader.c 3 Jul 2009 14:49:45 -0000 1.5
+++ services/objloader/current/src/objloader.c 9 Oct 2009 13:21:50 -0000
@@ -6,11 +6,11 @@
*
* =================================================================
* ####ECOSGPLCOPYRIGHTBEGIN####
* -------------------------------------------
* This file is part of eCos, the Embedded Configurable Operating System.
- * Copyright (C) 2005, 2008 Free Software Foundation, Inc.
+ * Copyright (C) 2005, 2008, 2009 Free Software Foundation, Inc.
*
* eCos is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License as published by the Free
* Software Foundation; either version 2 or (at your option) any later
* version.
@@ -79,10 +79,12 @@
}
void
cyg_ldr_delete_elf_section(PELF_OBJECT p, cyg_uint32 idx)
{
+ if (p->sections[idx] == 0)
+ return;
cyg_ldr_free(p->sections[idx]);
p->sections[idx] = 0;
}
// Frees all the memory allocated for a particular ELF object. Also calls
@@ -92,11 +94,11 @@
cyg_ldr_free_elf_object(PELF_OBJECT p)
{
cyg_int32 i;
for (i = 0; i < p->p_elfhdr->e_shnum + 1; i++)
- if (p->sections[i])
+ if (p->sections[i] != 0)
cyg_ldr_delete_elf_section(p, i);
if (p->sections != 0)
cyg_ldr_free(p->sections);
@@ -140,20 +142,31 @@
// Allocates memory and loads the contents of a specific ELF section.
// Returns the address of the newly allocated memory, of 0 for any error.
cyg_uint32
*cyg_ldr_load_elf_section(PELF_OBJECT p, cyg_uint32 idx)
{
- cyg_uint32 *addr = (cyg_uint32 *)cyg_ldr_malloc(p->p_sechdr[idx].sh_size);
- CYG_ASSERT(addr != 0, "Cannot malloc() section");
- if (addr == 0)
+ // Make sure we are not requesting the loading of a section for which we
+ // have no pointer.
+ CYG_ASSERT(idx < p->p_elfhdr->e_shnum + 1, "Invalid section id.");
+
+ // If this section has already been loaded its pointer is already available
+ // in the sections[] array.
+ if (p->sections[idx] != 0)
+ return p->sections[idx];
+ p->sections[idx] = (cyg_uint32)cyg_ldr_malloc(p->p_sechdr[idx].sh_size);
+ CYG_ASSERT(p->sections[idx] != 0, "Cannot malloc() section");
+ if (p->sections[idx] == 0)
{
cyg_ldr_last_error = "ERROR IN MALLOC";
return (void*)0;
}
p->seek(p, p->p_sechdr[idx].sh_offset);
- p->read(p, sizeof(char), p->p_sechdr[idx].sh_size, addr);
- return addr;
+ p->read(p,
+ sizeof(char),
+ p->p_sechdr[idx].sh_size,
+ (void *)p->sections[idx]);
+ return p->sections[idx];
}
// Returns the starting address of a section. If the section is not already
// loaded in memory, area for it will be allocated and the section will be
// loaded.
@@ -275,11 +288,13 @@
p->read(p, p->p_elfhdr->e_shentsize, p->p_elfhdr->e_shnum, p->p_sechdr);
// Load the section header string table. This is a byte oriented table,
// so alignment is not an issue.
idx = p->p_elfhdr->e_shstrndx;
- p->sections[idx] = cyg_ldr_load_elf_section(p, idx);
+ cyg_uint32 section_addr = cyg_ldr_load_elf_section(p, idx);
+ if (section_addr == 0)
+ return -1;
return 0;
}
PELF_OBJECT
cyg_ldr_open_library(CYG_ADDRWORD ptr, cyg_int32 mode)
@@ -337,11 +352,11 @@
// Now look for the index of .symtab. These are the symbols needed for
// the symbol retrieval as well as relocation.
if (!strcmp(p_shstrtab + e_obj->p_sechdr[i].sh_name, ELF_STRING_symtab))
{
e_obj->hdrndx_symtab = i;
- e_obj->sections[i] = cyg_ldr_load_elf_section(e_obj, i);
+ cyg_ldr_load_elf_section(e_obj, i);
if (e_obj->sections[i] == 0)
{
cyg_ldr_free_elf_object(e_obj);
return 0;
}
@@ -351,11 +366,11 @@
// to compare the name of external references symbols against the
// names in the in the CYG_HAL_TABLE provided by the user.
if (!strcmp(p_shstrtab + e_obj->p_sechdr[i].sh_name, ELF_STRING_strtab))
{
e_obj->hdrndx_strtab = i;
- e_obj->sections[i] = cyg_ldr_load_elf_section(e_obj, i);
+ cyg_ldr_load_elf_section(e_obj, i);
if (e_obj->sections[i] == 0)
{
cyg_ldr_free_elf_object(e_obj);
return 0;
}
Index: services/objloader/current/cdl/objloader.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/services/objloader/current/cdl/objloader.cdl,v
retrieving revision 1.6
diff -U5 -r1.6 objloader.cdl
--- services/objloader/current/cdl/objloader.cdl 3 Jul 2009 14:49:44 -0000 1.6
+++ services/objloader/current/cdl/objloader.cdl 9 Oct 2009 13:21:49 -0000
@@ -189,29 +189,27 @@
description "
This option enables the building of a library and an
application for testing the loader."
make -priority 320 {
- tests/testromfs/hello.o : <PACKAGE>/tests/library/hello.c
- @sh -c "mkdir -p tests tests/testromfs"
- $(CC) -c $(INCLUDE_PATH) -I$(dir $<) $(CFLAGS) -o $@ $<
+ testobj/hello.o : <PACKAGE>/tests/library/hello.c
+ @mkdir -p "$(dir $@)"
+ $(CC) -c $(INCLUDE_PATH) -I$(dir $<) $(ACTUAL_CFLAGS) -o $@ $<
}
make -priority 322 {
- <PREFIX>/include/cyg/objloader/testromfs_be.h : tests/testromfs/hello.o
- $(PREFIX)/bin/mk_romfs -b tests/testromfs testromfs_be.bin
- @mkdir -p "$(dir $@)"
- @tclsh $(PREFIX)/bin/file2c.tcl testromfs_be.bin testromfs_be.h
- @cp testromfs_be.h $@
+ <PREFIX>/include/cyg/objloader/testromfs_be.h : testobj/hello.o
+ $(PREFIX)/bin/mk_romfs -b $(dir $<) testromfs_be.bin
+ @mkdir -p "$(dir $@)"
+ @tclsh $(PREFIX)/bin/file2c.tcl testromfs_be.bin $@
}
make -priority 322 {
- <PREFIX>/include/cyg/objloader/testromfs_le.h : tests/testromfs/hello.o
- $(PREFIX)/bin/mk_romfs tests/testromfs testromfs_le.bin
- @mkdir -p "$(dir $@)"
- @tclsh $(PREFIX)/bin/file2c.tcl testromfs_le.bin testromfs_le.h
- @cp testromfs_le.h $@
+ <PREFIX>/include/cyg/objloader/testromfs_le.h : testobj/hello.o
+ $(PREFIX)/bin/mk_romfs $(dir $<) testromfs_le.bin
+ @mkdir -p "$(dir $@)"
+ @tclsh $(PREFIX)/bin/file2c.tcl testromfs_le.bin $@
}
cdl_option CYGPKG_OBJLOADER_TESTS {
display "Objloader tests"
flavor data
Index: fs/rom/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/rom/current/ChangeLog,v
retrieving revision 1.27
diff -U5 -r1.27 ChangeLog
--- fs/rom/current/ChangeLog 28 Apr 2009 13:06:11 -0000 1.27
+++ fs/rom/current/ChangeLog 9 Oct 2009 13:21:50 -0000
@@ -1,8 +1,14 @@
+2009-10-09 John Dallaway <john@dallaway.org.uk>
+
+ * cdl/romfs.cdl: Eliminate workarounds for file path handling
+ issue in obsolete Cygwin tclsh. Build the mk_romfs tool with higher
+ priority than the custom rules which use it.
+
2009-04-28 John Dallaway <john@dallaway.org.uk>
- cdl/romfs.cdl: Use CYGPKG_IO_FILEIO as the parent.
+ * cdl/romfs.cdl: Use CYGPKG_IO_FILEIO as the parent.
2009-02-07 John Dallaway <john@dallaway.org.uk>
* cdl/romfs.cdl: Pass file2c.tcl to tclsh directly.
Index: fs/rom/current/cdl/romfs.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/rom/current/cdl/romfs.cdl,v
retrieving revision 1.15
diff -U5 -r1.15 romfs.cdl
--- fs/rom/current/cdl/romfs.cdl 28 Apr 2009 13:06:11 -0000 1.15
+++ fs/rom/current/cdl/romfs.cdl 9 Oct 2009 13:21:50 -0000
@@ -67,16 +67,21 @@
cdl_option CYGBLD_FS_ROMFS_MK_ROMFS {
display "Build the tool used to build filesystems"
flavor bool
default_value 1
+ make -priority 98 {
+ <PREFIX>/bin/file2c.tcl: <PACKAGE>/support/file2c.tcl
+ @mkdir -p "$(dir $@)"
+ @cp $< $@
+ }
+
# FIXME: host compiler/flags should be provided by config system
- make -priority 100 {
- <PREFIX>/bin/mk_romfs: <PACKAGE>/support/mk_romfs.c <PREFIX>/bin/file2c.tcl
+ make -priority 98 {
+ <PREFIX>/bin/mk_romfs: <PACKAGE>/support/mk_romfs.c
@mkdir -p "$(dir $@)"
@$(HOST_CC) -g -O2 -o $@ $< || cc -g -O2 -o $@ $< || gcc -g -O2 -o $@ $<
- @cp $(REPOSITORY)/$(PACKAGE)/support/file2c.tcl $(PREFIX)/bin
}
description "
When enabled this option will build a host tool which can be
used to create a rom filesystem image."
@@ -110,28 +115,21 @@
requires CYGBLD_FS_ROMFS_MK_ROMFS
description "
This option enables the building of the ROM filesystem tests."
make -priority 100 {
- <PREFIX>/include/cyg/romfs/testromfs_le.h : <PREFIX>/bin/mk_romfs <PACKAGE>/support/file2c.tcl
- $(PREFIX)/bin/mk_romfs $(REPOSITORY)/$(PACKAGE)/tests/testromfs testromfs_le.bin
+ <PREFIX>/include/cyg/romfs/testromfs_le.h : <PACKAGE>/tests/testromfs <PREFIX>/bin/mk_romfs <PREFIX>/bin/file2c.tcl
+ $(PREFIX)/bin/mk_romfs $< testromfs_le.bin
@mkdir -p "$(dir $@)"
- # work around cygwin path problems by copying to build dir
- @cp $(REPOSITORY)/$(PACKAGE)/support/file2c.tcl .
- tclsh file2c.tcl testromfs_le.bin testromfs_le.h
- @rm -f $@ file2c.tcl
- @cp testromfs_le.h $@
+ tclsh $(PREFIX)/bin/file2c.tcl testromfs_le.bin $@
}
make -priority 100 {
- <PREFIX>/include/cyg/romfs/testromfs_be.h : <PREFIX>/bin/mk_romfs <PACKAGE>/support/file2c.tcl
- $(PREFIX)/bin/mk_romfs -b $(REPOSITORY)/$(PACKAGE)/tests/testromfs testromfs_be.bin
+ <PREFIX>/include/cyg/romfs/testromfs_be.h : <PACKAGE>/tests/testromfs <PREFIX>/bin/mk_romfs <PREFIX>/bin/file2c.tcl
+ $(PREFIX)/bin/mk_romfs -b $< testromfs_be.bin
@mkdir -p "$(dir $@)"
- # work around cygwin path problems by copying to bin dir
- @cp $(REPOSITORY)/$(PACKAGE)/support/file2c.tcl $(PREFIX)/bin/
- tclsh $(PREFIX)/bin/file2c.tcl testromfs_be.bin testromfs_be.h
- @cp testromfs_be.h $@
+ tclsh $(PREFIX)/bin/file2c.tcl testromfs_be.bin $@
}
cdl_option CYGPKG_FS_ROM_TESTS {
display "ROM filesystem tests"
flavor data