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 <[email protected]> + + * cdl/objloader.cdl: Eliminate workarounds for file path handling + issue in obsolete Cygwin tclsh. Use ACTUAL_CFLAGS for robustness. + +2009-09-13 Anthony Tonizzo <[email protected]> + + * 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 <[email protected]> * 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 <[email protected]> + + * 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 <[email protected]> - 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 <[email protected]> * 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
