Raar gedrag compiler.

Op 10 augustus 2020 18:58:05 schreef henri62:
Wat een achterlijke optimalisatie! Foei voor de gcc ontwikkelaars.

Ze zeggen dat het een "real world" verbetering oplevert.

Ik denk dat het gaat om gevallen als:

void somefunc (mytype *p)
{
  if (p == NULL) return; // of fout geven. 

  p->this = that;

  een_macro (p);
}

Die macro (of inline functie) kan je eventueel alleen voor als debugging aanstaat ook nog iets laten doen als:

if (p == NULL) return;
// doe iets met p. 

(even als de body van een inline functie geschreven niet het gedoe om een meer-regelige-macro te schrijven d'r bij gehaald. )

Hier ziet de compiler dus als p null was geweest, de this/that regel zou zijn gecrashed, dus de volgende if (p == NULL) hoeft dan niet meer gemaakt te worden. Zo vervalt er wat code.

Ze zeggen ook dat het niet makkelijk te vermijden is dat de warning triggert in code die toch nooit uitgevoerd kan worden.

@sparkygsx: Het nut van de debugger is beperkt.
Als ie de null dereference tegenkomt gooit ie een fault, en dan komt ie in "unhandled_fault_handler ()" terecht. Voor een paarhonderd bytes kan de maker van chibios daar wat nuttigs van maken, maar dat ding is waardeloos.

void unhandled_fault_handler (void)
{
struct debug {
   void *where;
   void *arg;
   uint32_t type; 
} dbg;
  dbg.where = <waar de hardware de IP opslaat> 
  dbg.arg = <addres causing fault> 
  dbg.type = <fault_cause>
  while (1);
}

Dan kan je in de debugger met print dbg zien waar de boel gecrasht is en welk adres de fout heeft getriggerd. Deze functie zit in chibios, ik krijg het niet voor mekaar om hem te overrulen in mijn code. Als ik IN chibios ga zitten wijzigen heb ik straks problemen als ik chibios wil upgraden of zijn m'n wijzigingen weer ongedaan gemaakt als ik "schoon" begin.

Daarnaast kan je runnen tot een bepaald punt en dan met step en next kijken wanneer ie weer crasht.

Dat werkt niet want na de eerste step dan zit ie in de timer interrupt ipv op de volgende regel in je code. Dus, ja de debugger hangt er soms aan, maar door dit soort dingen werkt dat minder prettig als zou moeten.

Update: Binnen GCC lijkt er een productieve discussie ontstaan te zijn over deze warning. Er gaan stemmen op om te proberen hem aan te zetten: Er zijn nog "maar" 700 plekken in gcc waar de code iets aangepast moet worden om de warning niet te triggeren.....

Er wordt ook gesproken over may/must. Ik denk dat mijn code dan een MUST geval is: De code-flow MOET de null dereference tegenkomen. Terwijl in veel andere gevallen er een "may" is dat er een takje van de code toevallig iets raars doet waardoor de warning zou triggeren maar dat dan niet de hele code nooit meer verder kan.

[Bericht gewijzigd door rew op dinsdag 11 augustus 2020 08:35:28 (12%)

Als ie in de hardfault zit kun je ook met je debugger de CPU registers lezen, dat staat precies de informatie die je zoekt. Als je de timer niet door laat lopen terwijl je in een breakpoint zit (dat is eigenlijk alleen nodig met live power electronics eraan), loopt die op dezelfde interval in cycles als normaal, dus hij zou niet constant in de interrupt moeten komen. Voor dit specifieke probleem had je die hele interrupt ook uit kunnen zetten.

Alsnog was het handig geweest als je een warning zit krijgen als de compiler zo'n substantieel stuk code weggooit; een enkele check zijn maar een paar instructies, zodra het meer wordt is een warning wel op zijn plaats, niemand schrijft zoveel code zonder een bedoeling.

Nouja, je hebt wel eens dat je om te testen een

  while (1) {
     toggle_led (); 
     delay (100ms); 
  }

ergens in je main om zeker te weten dat ie daar aankomt. dat maakt alles wat er achter staat dode code en kan ie voor deze keer wegoptimaliseren. In het onderhavige geval merkte ik dus dat er wat fout ging doordat het programmeren veel sneller ging dan verwacht. Dat kan ook een voordeel zijn als je het WEL bedoeld had.

(in mijn huidige project zou ik bijvoorbeeld misschien de hele LWIP willen weglaten en alleen met i2c wat willen doen als ik een probleem in het i2c stuk aan het debuggen ben.)

Ja, ik heb het een keer uitgezocht waar die hardware registers zitten. Gewoon googlen en dan kom je ergens midden in een ARM architecture manual en daar staat het. Niet onthouden want met eenvoudige google kan je het vinden en 2e en derde keer kan ik het niet vinden. Register bestaat niet in mijn ARM, zit ik op een ARMv7 manual terwijl dit een ARMv6 is. En ik krijg die manuals niet genavigeerd naar dezelfde sectie voor de andere ARM. Paar keer geprobeerd. Opgegeven. Ik zal wel stom zijn.

Dat de clocks stil kunnen als de CPU stopt op een breakpunt weet ik ook. Hoe dat moet: nog niet uitgezocht.

Dat het moeilijk is komt doordat je zo eigenwijs bent om geen IDE te gebruiken; in Atollic voor STM32 bijvoorbeeld hoef je alleen het registers paneel te openen en daar staat alles wat je zoekt.

Als je dat vertikt, acht jij je tijd blijkbaar niet zoveel waard.

Aan dat voorbeeld van void somefunc (mytype *p) {...
valt niets aan te optimaliseren als je die in een externe file zet.
Het is een runtime check.

Alleen als de functie inlined wordt, zou je de regel "if (p == NULL) return;" in zijn geheel weg kunnen laten als de compiler ZEKER weet dat p geen NULL is. Dat gebeurd dan eigenlijk alleen als de p een const is oid.

Verder is het een totaal zinloze actie om daar optimalisaties op los te laten.
Zoals ik al zei, de ontwikkelaars bij de gcc community zijn volgens mij gewoon dingen aan het verzinnen om eh ... dingen te verzinnen ...
Compleet nutteloos en in veel gevallen puur om maar weer een nieuwe versie te kunnen releasen. Ik zou zegge tegen die gasten: Implementeer nu eens iets wat echt nuttig is of stop gewoon met die fratsen. Goed is goed, alleen compiler bugfixes, thats it.

Nou dat is ook wel kort door de bocht; optimalisaties zijn echt wel nuttig, en er zijn wel meer gevallen waar die check echt overbodig is.

In dit geval had de compiler ook gelijk; de code na de assignment was onbereikbaar (mits een null pointer dereference een trap geeft), en bij gevolg is de assignment zelf ook niet nuttig meer.

Stel je voor dat we bij emissienormen voor auto's in 1980 hadden gezegd: "is wel goed zo, niets meer aan doen".

Ik heb het topic niet helemaal doorgelezen maar probeer misschien eens de memset weg te laten en de (struct/union/...) (verborgen achter de typedef) op de volgende manier op 0 te zetten:

int main (void)
{
  lwipthread_opts_t opts = { 0 };

  ... // wat initializatie dingen. 
  opts.macaddress[0] = 0x86;

  lwipInit(&opts);

 ... rest van m'n code. 

Ik vind dit persoonlijk leesbaarder en nu zal de compiler ook alles op 0 zetten en niet de implementatie van de standaard C bibliotheek. Misschien is het namelijk geen compiler bug maar een bug in de bibliotheek implementatie.

Update: ik heb net eens naar de chibios code gekeken wat die struct nu is:

/**
 * @brief   Runtime TCP/IP settings.
 */
typedef struct lwipthread_opts {
  /**
   * @brief   Pointer to MAC address as an array of 6 unsigned bytes.
   */
  uint8_t         *macaddress;
  /**
   * @brief   Network address as 32-bit unsigned integer.
   */
  uint32_t        address;
  /**
   * @brief   Network subnet mask as 32-bit unsigned integer.
   */
  uint32_t        netmask;
  /**
   * @brief   Network gateway as 32-bit unsigned integer.
   */
  uint32_t        gateway;
  /**
   * @brief   Startup network addressing mode - static, DHCP, auto.
   */
  net_addr_mode_t addrMode;
  /**
   * @brief   Host name. If NULL, a default string is used.
   * @note    Not checked for validity. In particular, spaces not allowed.
   */
#if LWIP_NETIF_HOSTNAME || defined(__DOXYGEN__)
  const char              *ourHostName;
#endif
  /**
   * @brief   Link up callback.
   *
   * @note    Called from the tcpip thread when the link goes up.
   *          Can be NULL to default to lwipDefaultLinkUpCB.
   */
  void (*link_up_cb)(void*);
  /**
   * @brief   Link down callback.
   *
   * @note    Called from the tcpip thread when the link goes down.
   *          Can be NULL to default to lwipDefaultLinkDownCB.
   */
  void (*link_down_cb)(void*);
} lwipthread_opts_t;

Dit verklaart dus ook meteen waarom wat je doet niet werkt. Je gaat daar een waarde zetten op een array die niet gealloceerd is. Je moet dus het volgende doen:

/* MAC address is 48 bits lang */
static uint8_t macaddr[] = { 0x86, 0x86, 0x86, 0x86, 0x86, 0x86 };

int main (void)
{
  lwipthread_opts_t opts = { 0 };


  ... // wat initializatie dingen. 
  opts.macaddress = macaddr;

  lwipInit(&opts);

 ... rest van m'n code. 

De compiler zal zich hierop stukgebeten hebben.

Mvg,
Daan