Allow to use as a library

[mcspr]

Hey,

About a month ago I was trying to unit testing support to an esp8266 project and it turned out to be quite easy to setup this repo as a base for Arduino emulation layer. But, because Arduino.cpp file defines main(), it is not possible to use it as a library (e.g., see this configuration file for PlatformIO - https://github.com/xoseperez/espurna/blob/dev/code/test/platformio.ini)

It is easily solved by adding __attribute__((weak)) for main()
Also, in case someone would want to call this main() anyway, define the existing symbol by another name so it does not get stripped and user code can call __main()

PS. I did see the README notice :)

[bxparks]

Thanks for the PR, this is a great idea.

I did some testing and I found that __attribute__((weak, alias(".."))) works on g++7/Linux, and clang++8/Linux, but is not supported on clang++11/Darwin (i.e. MacOS). The error message is:

error: aliases are not supported on darwin
___attribute__((weak, alias("__main")));

I think the workaround is to remove the alias() part. Because as far as I can tell, the purpose of the (alias()) is to point a large number of weak functions to a single strong function, to save on generated code. Since we have only a single weak function, we don't really need it.

The code that worked on both Linux and Mac, on both g++ and clang++ was:

extern "C"
int main(int argc, char** argv)
__attribute__((weak));                          
  
int main(int argc, char** argv) {
  signal(SIGINT, handleControlC);
  atexit(disableRawMode);
  enableRawMode();                                                              
  
  setup();
  while (true) {                                                                
    char c = '\0';                                                              
    read(STDIN_FILENO, &c, 1);                                                  
    if (c) Serial.insertChar(c);                                                
    loop();
    yield();
  }
} 

Can you verify that this new version works for you? If so, we can check that in.

(BTW, I notice a mix of spaces and tabs in this file. That was not intentional, I use only spaces for indent. I will fix that after you submit your code.)

[bxparks]

Oh, re-reading your comments, I see that the other reason for delegating to __main() is to allow someone to call it directly. So how about:

extern "C"
int main(int argc, char** argv)
__attribute__((weak));

int main(int argc, char** argv) {
  return __main(argc, argv):
}

extern "C"
int __main(int argc, char** argv) {
  ...
}

[mcspr]

Makes sense, but I have another issue with that approach. Here are a couple of builds to showcase compilation errors when adding dummy pinMode(...) calls, causing Arduino.o to be linked into the binary.
I'd like to replace the current patch with the last one, I hope comment above + (copyright) move is fine?

• When only main() is weak:
  mcspr@6c217ff
  https://travis-ci.com/mcspr/espurna/jobs/271692319#L359-L361

.pio/build/test/libe77/libUnixHostDuino.a(Arduino.o): In function `__main(int, char**)':
Arduino.cpp:(.text+0x38c): undefined reference to `setup'
Arduino.cpp:(.text+0x3c6): undefined reference to `loop'
collect2: error: ld returned 1 exit status

• Weak declarations for loop() and setup(), runtime linker will fail instead
  (I renamed my test's main() to make it use the main from Arduino.cpp)
  mcspr@250ebb7
  https://travis-ci.com/mcspr/espurna/jobs/271692811#L348
  Platformio hides error though, but essentially it just wont work at all:

$ .pio/build/test/program
fish: “.pio/build/test/program” terminated by signal SIGSEGV (Address boundary error)

• Using another compilation unit successfully builds the app:
  mcspr@076cd9f
  https://travis-ci.com/mcspr/espurna/jobs/271692872
  And there is still linker error when intentionally using main() from here, but not adding setup() and loop():
  https://travis-ci.com/mcspr/espurna/jobs/271693136#L342-L344

---

From what I remember, extern "C" was added for alias specifically. Since none of the patches above use extern and still compile and work just fine, I would guess it is not needed here.

[bxparks]

Hi,
(It took me a bit of effort to figure out that I should click through the mcspr@xxxxx links to see the code fragments that you were referring to!)

Your last patch creates a main.cpp, and converts __main() into an inline function. But I think this makes __main() invisible to any outside code. Which I think is fine. But if __main() is not usable, we can achieve the same result by simply using a single main(). So I propose going back to the code I posted in my first reply, (and not create a main.cpp):

Arduino.cpp:

extern "C"
int main(int argc, char** argv)
__attribute__((weak));                          
  
int main(int argc, char** argv) {
  signal(SIGINT, handleControlC);
  atexit(disableRawMode);
  enableRawMode();                                                              
  
  setup();
  while (true) {                                                                
    char c = '\0';                                                              
    read(STDIN_FILENO, &c, 1);                                                  
    if (c) Serial.insertChar(c);                                                
    loop();
    yield();
  }
} 

If I understand things correctly, if your PlatformIO code provides its own main(), it will clobber this one, and the references to loop() and setup() will be removed, which fixes the linker problem. Yes?

[mcspr]

Yes! Sorry for confusion.
I think inline is not intended there... Otherwise, that patch is the only way it builds with that configuration. Simply adding weak attribute to the main has the same effect as having weak main and __main, despite main being weak and supposedly be unused.

I wonder if object link order matters somehow? After platformio builds all packages:

g++ -o .pio/build/test/program .pio/build/test/test/output_export.o .pio/build/test/test/runner.o -L.pio/build/test -Wl,--start-group .pio/build/test/lib6e3/libUnixHostDuino.a .pio/build/test/libaf3/libStreamString.a .pio/build/test/libUnityTestLib.a -Wl,--end-group

I forgot to actually push the patch here, so I'll do that now.

[mcspr]

The main difference is that there is no inline anymore, since it did not do the thing I though it will do. Without optimisation options symbol is still there, but with optimisation it disappears and causes link error when trying to call it.
And __ seems to be for compiler reserved names, plus looks nicer

[bxparks]

Ok, thanks, I'm going to merge your stuff in. I can do some small fixes afterwards (like some trailing spaces). I'm pretty sure that the extern "C" is not necessary, because main() will never be called by a C program directly.

[bxparks]

Looks like I need to keep the extern "C" in main.cpp to match the declarations in Arduino.h which wraps the whole bunch in extern "C" { }.

[mcspr]

Thanks a lot!

As far as I understood the extern "C" stuff while declared like that needs slightly more changes; e.g. if in my case I write some test file as .c and include Arduino.h (but that now fails with Print class code anyway)