Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions arch/sim/src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,15 @@ endif

# Override in Make.defs if linker is not 'ld'

LDSTARTGROUP ?= --start-group
LDENDGROUP ?= --end-group
ifeq ($(HOSTOS),Darwin)
LDSTARTGROUP ?=
LDENDGROUP ?=
LDUNEXPORTSYMBOLS ?= -unexported_symbols_list $(HOSTOS)-names.dat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so, why we don't update other platform to use -unexported_symbols_list? to simplify the logic in Makefile.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do gnu ld have the option? my copy of its documentation doesn't have it.

Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Jan 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emm, unexported_symbols_list doesn't support on Linux, but after searching we can install binutils and use gobjcopy? even for ld so we don't modify Makefile at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as i mentioned in the commit logs, gnu binutils didn't actually work for me.
objcopy runs without errors, producing a corrupted object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

else
LDSTARTGROUP ?= --start-group
LDENDGROUP ?= --end-group
LDUNEXPORTSYMBOLS ?=
endif

EXTRA_LIBS ?=
EXTRA_LIBPATHS ?=
Expand Down Expand Up @@ -292,10 +299,15 @@ else
$(Q) cp nuttx-names.dat $@
endif

Darwin-names.dat: nuttx-names.dat
$(Q) cat $^ | sed -e "s/^\([^[:space:]][^[:space:]]*\)[[:space:]].*/_\1/g" >$@

nuttx.rel : libarch$(LIBEXT) board/libboard$(LIBEXT) $(HOSTOS)-names.dat $(LINKOBJS)
$(Q) echo "LD: nuttx.rel"
$(Q) $(LD) -r $(LDLINKFLAGS) $(RELPATHS) $(EXTRA_LIBPATHS) -o $@ $(REQUIREDOBJS) $(LDSTARTGROUP) $(RELLIBS) $(EXTRA_LIBS) $(LDENDGROUP)
$(Q) $(LD) -r $(LDLINKFLAGS) $(RELPATHS) $(EXTRA_LIBPATHS) -o $@ $(REQUIREDOBJS) $(LDSTARTGROUP) $(RELLIBS) $(EXTRA_LIBS) $(LDENDGROUP) $(LDUNEXPORTSYMBOLS)
ifneq ($(HOSTOS),Darwin)
$(Q) $(OBJCOPY) --redefine-syms=$(HOSTOS)-names.dat $@
endif

# Generate the final NuttX binary by linking the host-specific objects with the NuttX
# specific objects (with munged names)
Expand All @@ -306,7 +318,6 @@ nuttx$(EXEEXT): cleanrel nuttx.rel $(HOSTOBJS)
$(Q) $(NM) $(TOPDIR)/$@ | \
grep -v '\(compiled\)\|\(\.o$$\)\|\( [aUw] \)\|\(\.\.ng$$\)\|\(LASH[RL]DI\)' | \
sort > $(TOPDIR)/System.map
$(Q) rm -f nuttx.rel

# This is part of the top-level export target

Expand All @@ -327,7 +338,7 @@ export_startup: board/libboard$(LIBEXT) up_head.o $(HOSTOBJS)
depend: .depend

cleanrel:
$(Q) rm -f nuttx.rel GNU/Linux-names.dat Cygwin-names.dat
$(Q) rm -f nuttx.rel $(HOSTOS)-names.dat

clean: cleanrel
$(Q) if [ -e board/Makefile ]; then \
Expand Down
2 changes: 1 addition & 1 deletion arch/sim/src/sim/up_initialstate.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,6 @@
void up_initial_state(struct tcb_s *tcb)
{
memset(&tcb->xcp, 0, sizeof(struct xcptcontext));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we add a dummy field in xcptcontext instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't understand your suggestion. can you explain?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the below dummy field:
struct xcptcontext
{
void sigdeliver; / Actual type is sig_deliver_t */
xcpt_reg_t dummy;
xcpt_reg_t regs[XCPTCONTEXT_REGS];
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is about the actual stack alignment. that is, the value of RSP register.
not about the context structure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My previous comment is wrong, but all places(up_use_stack/up_stack_frame) which modify adj_stack_ptr already align this pointer to 16B boundary, the extra subtraction just make the situation even worse, I can't understand why this change fix your problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the convention is, align before a function call.
that is, from POV of the function being called, it will be aligned after pushing RBP.
note that up_longjmp jumps into the function directly. (without pushing the return address)
RSP at that point should not be aligned.

tcb->xcp.regs[JB_SP] = (xcpt_reg_t)tcb->adj_stack_ptr;
tcb->xcp.regs[JB_SP] = (xcpt_reg_t)tcb->adj_stack_ptr - sizeof(xcpt_reg_t);
tcb->xcp.regs[JB_PC] = (xcpt_reg_t)tcb->start;
}
12 changes: 8 additions & 4 deletions arch/sim/src/sim/up_setjmp32.S
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,20 @@
#ifdef __CYGWIN__
# define SYMBOL(s) _##s
#else
#ifdef __ELF__
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is wrong, should it be #ifndef?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, you are right.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# define SYMBOL(s) _##s
#else
# define SYMBOL(s) s
#endif
#endif

/**************************************************************************
* Public Functions
**************************************************************************/

.text
.globl SYMBOL(up_setjmp)
#ifndef __CYGWIN__
#ifdef __ELF__
.type SYMBOL(up_setjmp), @function
#endif
SYMBOL(up_setjmp):
Expand Down Expand Up @@ -86,11 +90,11 @@ SYMBOL(up_setjmp):

xorl %eax, %eax
ret
#ifndef __CYGWIN__
#ifdef __ELF__
.size SYMBOL(up_setjmp), . - SYMBOL(up_setjmp)
#endif
.globl SYMBOL(up_longjmp)
#ifndef __CYGWIN__
#ifdef __ELF__
.type SYMBOL(up_longjmp), @function
#endif
SYMBOL(up_longjmp):
Expand All @@ -112,7 +116,7 @@ SYMBOL(up_longjmp):
/* Jump to saved PC. */

jmp *%edx
#ifndef __CYGWIN__
#ifdef __ELF__
.size SYMBOL(up_longjmp), . - SYMBOL(up_longjmp)
#endif

12 changes: 8 additions & 4 deletions arch/sim/src/sim/up_setjmp64.S
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,11 @@
//# define SYMBOL(s) _##s
# define SYMBOL(s) s
#else
#ifdef __ELF__
# define SYMBOL(s) s
#else
# define SYMBOL(s) _##s
#endif
#endif

/**************************************************************************
Expand All @@ -91,7 +95,7 @@
.text
.align 4
.globl SYMBOL(up_setjmp)
#ifndef __CYGWIN__
#ifdef __ELF__
.type SYMBOL(up_setjmp), @function
#endif
SYMBOL(up_setjmp):
Expand Down Expand Up @@ -128,13 +132,13 @@ SYMBOL(up_setjmp):

ret

#ifndef __CYGWIN__
#ifdef __ELF__
.size SYMBOL(up_setjmp), . - SYMBOL(up_setjmp)
#endif

.align 4
.globl SYMBOL(up_longjmp)
#ifndef __CYGWIN__
#ifdef __ELF__
.type SYMBOL(up_longjmp), @function
#endif
SYMBOL(up_longjmp):
Expand All @@ -157,7 +161,7 @@ SYMBOL(up_longjmp):

jmp *JB_RSI(REGS) /* Save 8: rsi */

#ifndef __CYGWIN__
#ifdef __ELF__
.size SYMBOL(up_longjmp), . - SYMBOL(up_longjmp)
#endif

2 changes: 1 addition & 1 deletion arch/sim/src/sim/up_stackframe.c
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ FAR void *up_stack_frame(FAR struct tcb_s *tcb, size_t frame_size)

/* Reset the initial state */

tcb->xcp.regs[JB_SP] = (xcpt_reg_t)tcb->adj_stack_ptr;
tcb->xcp.regs[JB_SP] = (xcpt_reg_t)tcb->adj_stack_ptr - sizeof(xcpt_reg_t);

/* And return a pointer to the allocated memory */

Expand Down
2 changes: 1 addition & 1 deletion binfmt/binfmt_globals.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
* protection to simply disable pre-emption when accessing this list.
*/

FAR struct binfmt_s *g_binfmts;
FAR struct binfmt_s *g_binfmts = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use -fno-common option instead? There are many place which don't initialize global variables explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it works. do you prefer it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many places which don't set the global variable to NULL, the same error will report again and again if we turn more option in defconfig. The right fix is to add -fno-common.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, let me try the -fno-common approach later.


/****************************************************************************
* Private Functions
Expand Down
19 changes: 17 additions & 2 deletions boards/sim/sim/sim/README.txt
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,12 @@ The simulation build is a two pass build:
created called nuttx.rel. This includes all of the files that are part
of the NuttX "domain."

2. On the second pass, the files are are in the host OS domain are build
2. On the second pass, the files which are in the host OS domain are build
and then linked with nuttx.rel to generate the simulation program.

NuttX is a POSIX compliant RTOS and is normally build on a POSIX compliant
host environment (like Linux or Cygwin). As a result, the same symbols are
exported by both the NuttX doman and the host domain. How can we keep them
exported by both the NuttX domain and the host domain. How can we keep them
separate?

This is done using the special file nuttx-name.dat. This file just contains
Expand All @@ -181,6 +181,10 @@ which version of strlen() you call. Other times, it can cause subtle,
mysterious errors. Usually, however, callng the wrong function in the wrong
OS results in a fatal crash.

On macOS, instead of objcopy, -unexported_symbols_list linker option is
used to hide symbols in the NuttX domain, using the same list of symbols
from nuttx-name.dat.

Networking Issues
-----------------
I never did get networking to work on the sim target. It tries to use the
Expand Down Expand Up @@ -487,6 +491,17 @@ Common Configuration Information
CONFIG_SIM_X8664_SYSTEMV=n
CONFIG_SIM_M32=n

g. macOS, 64-bit, 64-bit build

CONFIG_HOST_LINUX=n
CONFIG_HOST_MACOS=y
CONFIG_HOST_WINDOWS=n
CONFIG_HOST_X86=n
CONFIG_HOST_X86_64=y
CONFIG_SIM_X8664_MICROSOFT=n
CONFIG_SIM_X8664_SYSTEMV=y
CONFIG_SIM_M32=n

Configuration Sub-Directories
-----------------------------

Expand Down
4 changes: 1 addition & 3 deletions boards/sim/sim/sim/configs/cxxtest/Make.defs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
include ${TOPDIR}/.config
include ${TOPDIR}/tools/Config.mk

HOSTOS = ${shell uname -o 2>/dev/null || echo "Other"}
HOSTOS = ${shell uname -o 2>/dev/null || uname -s 2>/dev/null || echo "Other"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we fix the same issue in:
tools/define.sh
tools/incdir.sh
boards/arm/str71x/olimex-strp711/scripts/oocd.sh
arch/x86/src/Makefile

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess it doesn't actually matter as they seem just checking if cygwin or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right.


ifeq ($(CONFIG_DEBUG_SYMBOLS),y)
ARCHOPTIMIZATION = -g
Expand Down Expand Up @@ -116,9 +116,7 @@ EXTRA_LIBPATHS = -L "${shell dirname "$(LIBSUPXX)"}"
EXTRA_LIBS = -lsupc++

ifeq ($(CONFIG_DEBUG_SYMBOLS),y)
LDLINKFLAGS += -g
CCLINKFLAGS += -g
LDFLAGS += -g
endif

ifeq ($(CONFIG_SIM_M32),y)
Expand Down
4 changes: 1 addition & 3 deletions boards/sim/sim/sim/configs/nsh2/Make.defs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
include ${TOPDIR}/.config
include ${TOPDIR}/tools/Config.mk

HOSTOS = ${shell uname -o 2>/dev/null || echo "Other"}
HOSTOS = ${shell uname -o 2>/dev/null || uname -s 2>/dev/null || echo "Other"}

ifeq ($(CONFIG_DEBUG_SYMBOLS),y)
ARCHOPTIMIZATION = -g
Expand Down Expand Up @@ -108,9 +108,7 @@ CCLINKFLAGS = $(ARCHSCRIPT) # Link flags used with $(CC)
LDFLAGS = $(ARCHSCRIPT) # For backward compatibility, same as CCLINKFLAGS

ifeq ($(CONFIG_DEBUG_SYMBOLS),y)
LDLINKFLAGS += -g
CCLINKFLAGS += -g
LDFLAGS += -g
endif

ifeq ($(CONFIG_SIM_M32),y)
Expand Down
4 changes: 1 addition & 3 deletions boards/sim/sim/sim/scripts/Make.defs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
include ${TOPDIR}/.config
include ${TOPDIR}/tools/Config.mk

HOSTOS = ${shell uname -o 2>/dev/null || echo "Other"}
HOSTOS = ${shell uname -o 2>/dev/null || uname -s 2>/dev/null || echo "Other"}

ifeq ($(CONFIG_WINDOWS_MSYS),y)
DIRLINK = $(TOPDIR)/tools/copydir.sh
Expand Down Expand Up @@ -115,9 +115,7 @@ CCLINKFLAGS = $(ARCHSCRIPT) # Link flags used with $(CC)
LDFLAGS = $(ARCHSCRIPT) # For backward compatibility, same as CCLINKFLAGS

ifeq ($(CONFIG_DEBUG_SYMBOLS),y)
LDLINKFLAGS += -g
CCLINKFLAGS += -g
LDFLAGS += -g
endif

ifeq ($(CONFIG_SIM_M32),y)
Expand Down
4 changes: 3 additions & 1 deletion boards/sim/sim/sim/src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@

include $(TOPDIR)/Make.defs

# Add dummy.c to ensure that we have at least one object.
# On some platforms like macOS, we can't create an empty archive.
ASRCS =
CSRCS =
CSRCS = dummy.c

ifeq ($(CONFIG_BOARD_LATE_INITIALIZE),y)
CSRCS += sim_boot.c
Expand Down
Empty file.
2 changes: 1 addition & 1 deletion boards/x86/qemu/qemu-i486/scripts/Make.defs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
include ${TOPDIR}/.config
include ${TOPDIR}/tools/Config.mk

HOSTOS = ${shell uname -o 2>/dev/null || echo "Other"}
HOSTOS = ${shell uname -o 2>/dev/null || uname -s 2>/dev/null || echo "Other"}

ifeq ($(CONFIG_DEBUG_SYMBOLS),y)
ARCHOPTIMIZATION = -g
Expand Down
2 changes: 1 addition & 1 deletion mm/umm_heap/umm_globals.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,5 @@
#else
/* Otherwise, the user heap data structures are in common .bss */

struct mm_heap_s g_mmheap;
struct mm_heap_s g_mmheap = {};
#endif
2 changes: 1 addition & 1 deletion tools/Makefile.host
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ include ${TOPDIR}/tools/Config.mk
# Define HOSTCC on the make command line if it differs from these defaults
# Define HOSTCFLAGS with -g on the make command line to build debug versions

HOSTOS = ${shell uname -o || echo "Other"}
HOSTOS = ${shell uname -o 2>/dev/null || uname -s 2>/dev/null || echo "Other"}

ifeq ($(HOSTOS),MinGW)

Expand Down
4 changes: 2 additions & 2 deletions tools/sethost.sh
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,9 @@ echo " Refreshing..."
cd $nuttx || { echo "ERROR: failed to cd to $nuttx"; exit 1; }
make clean_context 1>/dev/null 2>&1
if [ "X${debug}" = "Xy" ]; then
make olddefconfig V=1
make olddefconfig V=1 || { echo "ERROR: failed to refresh"; exit 1; }
else
make olddefconfig 1>/dev/null 2>&1
make olddefconfig 1>/dev/null || { echo "ERROR: failed to refresh"; exit 1; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need keep 2>&1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's intentional to expose the errors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

fi

# Move config file to correct location and restore any previous .config
Expand Down