Add portable FW_UNREACHABLE macro for FW_ASSERT

[devin-ai-integration]

Related Issue(s)	nasa#5228
Has Unit Tests (y/n)	n
Documentation Included (y/n)	n
Generative AI was used in this contribution (y/n)	AI

---

Change Description

Adds two portable macros and applies them to all four FW_ASSERT variants:

• FW_UNREACHABLE() — general-purpose unreachable hint, portable across compilers:

  1. __has_builtin(__builtin_unreachable) — Clang, GCC ≥ 10
  2. defined(__GNUC__) — older GCC (4.5+) that lack __has_builtin
  3. defined(_MSC_VER) — MSVC, mapped to __assume(0)
  4. Fallback — ((void)0) no-op

• FW_ASSERT_UNREACHABLE() — the hint used inside FW_ASSERT macros. Expands to FW_UNREACHABLE() only when FW_ASSERTIONS_ALWAYS_ABORT is enabled; ((void)0) otherwise. This avoids undefined behavior in the default configuration where SwAssert can legally return.

The FW_NO_ASSERT path is unchanged.

Rationale

nasa#5228 adds __builtin_unreachable() directly in the FW_ASSERT macros. That built-in is GCC/Clang-specific, and using it unconditionally causes UB when FW_ASSERTIONS_ALWAYS_ABORT is 0 (the default), since SwAssert can return via AssertHook::doAssert().

This changeset:

1. Wraps the intrinsic in a portable FW_UNREACHABLE() macro with #ifdef guards
2. Gates usage in FW_ASSERT on FW_ASSERTIONS_ALWAYS_ABORT via FW_ASSERT_UNREACHABLE()
3. Fixes a line-continuation bug in the first FW_ASSERT variant of the original PR

Testing/Review Recommendations

• Verify the FW_UNREACHABLE() detection cascade covers your target toolchains.
• When FW_ASSERTIONS_ALWAYS_ABORT is enabled, the compiler gets the unreachable hint for better optimization/analysis. When disabled (default), behavior is unchanged from baseline.

Future Work

None.

AI Usage (see policy)

AI was used for code generation and review analysis.

IAMAI

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/d571fdaf3892412593f720adc160e783

---

[devin-ai-integration]

Original prompt from thomas.boyer.chammard

Please review nasa#5228
Also we'd like to make this portable across compilers, so we likely need an #ifdef that defines to a no-op if the macro isn't available?
Make a changeset for that please

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR that start with 'DevinAI' or '@devin'.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[github-actions]

Coverage report — base devel

No baseline branch coverage/devel found. This run becomes the seed once it lands on devel.

Overall (line): 81.12% (no baseline)
Regression threshold: 0.50% (line).

Regressions

(none over threshold)

Modules changed

(no measurable change)

New modules

Module	Line	Function	Branch
CFDP/Checksum	71.15	57.14	53.85
Drv/AsyncByteStreamBufferAdapter	100.00	100.00	100.00
Drv/ByteStreamBufferAdapter	100.00	100.00	100.00
Drv/Ip	44.32	57.38	25.36
Drv/TcpClient	75.00	100.00	47.37
Drv/TcpServer	84.72	100.00	60.00
Drv/Udp	68.09	90.91	42.86
Fw/Buffer	81.25	89.47	58.62
Fw/DataStructures	98.48	97.14	83.21
Fw/Dp	94.83	96.67	95.95
Fw/FilePacket	75.24	89.06	54.30
Fw/Log	75.71	84.21	60.71
Fw/Logger	100.00	100.00	100.00
Fw/SerializableFile	90.00	100.00	79.41
Fw/Time	88.62	85.48	86.84
Fw/Tlm	53.76	60.00	37.50
Fw/Types	53.84	56.01	34.32
Os	18.05	19.48	14.52
Os/Generic	89.10	96.30	67.18
Os/Generic/Types	92.45	91.67	82.14
Os/Posix	62.40	84.21	44.10
Svc/ActiveRateGroup	100.00	100.00	92.31
Svc/ActiveTextLogger	79.05	90.00	71.23
Svc/AssertFatalAdapter	94.74	100.00	86.67
Svc/BufferAccumulator	88.00	94.12	74.49
Svc/BufferLogger	92.62	86.96	80.46
Svc/BufferManager	99.05	100.00	83.64
Svc/BufferRepeater	91.67	100.00	75.00
Svc/ChronoTime	100.00	100.00	100.00
Svc/CmdDispatcher	96.97	91.67	91.75
Svc/CmdSequencer	93.89	97.12	84.63
Svc/CmdSplitter	100.00	100.00	100.00
Svc/ComLogger	97.37	91.67	83.91
Svc/ComSplitter	100.00	100.00	100.00
Svc/ComStub	98.51	100.00	85.19
Svc/DpCatalog	78.00	100.00	66.27
Svc/DpManager	97.44	100.00	100.00
Svc/DpWriter	97.58	90.00	97.22
Svc/FileDownlink	84.15	90.91	71.90
Svc/FileManager	88.41	93.33	82.44
Svc/FileUplink	92.35	96.55	80.95
Svc/FileWorker	90.29	100.00	84.87
Svc/FprimeDeframer	100.00	100.00	97.92
Svc/FprimeFramer	100.00	100.00	95.45
Svc/FprimeRouter	88.89	100.00	78.26
Svc/FpySequencer	86.76	99.04	76.89
Svc/GenericHub	100.00	100.00	85.64
Svc/Health	100.00	100.00	88.66
Svc/LinuxTimer	97.06	100.00	82.35
Svc/OsTime	70.00	83.33	60.98
Svc/PassiveRateGroup	100.00	100.00	89.47
Svc/PolyDb	100.00	100.00	53.57
Svc/PosixTime	100.00	100.00	75.00
Svc/PrmDb	92.75	89.47	88.24
Svc/RateGroupDriver	100.00	100.00	68.75
Svc/SeqDispatcher	73.08	80.00	71.05
Svc/StaticMemory	100.00	100.00	100.00
Svc/SystemResources	98.63	100.00	76.19
Svc/TlmChan	77.59	85.71	63.03
Svc/TlmPacketizer	92.18	100.00	77.45
Svc/Version	96.10	100.00	86.96
Utils	20.04	26.44	24.07
Utils/Types	92.11	95.83	77.08

Modules without UTs

CFDP/Checksum/GTest, Drv/ByteStreamDriverModel, Drv/Interfaces, Drv/LinuxGpioDriver, Drv/LinuxI2cDriver, Drv/LinuxSpiDriver, Drv/LinuxUartDriver, Drv/Ports, Drv/Ports/DataTypes, FppTestProject/FppTest/interfaces, FppTestProject/FppTest/topology/async, FppTestProject/FppTest/topology/components/Comp, FppTestProject/FppTest/topology/components/Framework, FppTestProject/FppTest/topology/components/Receiver, FppTestProject/FppTest/topology/components/Sender, FppTestProject/FppTest/topology/guarded, FppTestProject/FppTest/topology/ports, FppTestProject/FppTest/topology/sync, FppTestProject/FppTest/topology/top_ports, FppTestProject/FppTest/topology/types, Fw/Cmd, Fw/Com, Fw/Comp, Fw/FilePacket/GTest, Fw/Fpy, Fw/Interfaces, Fw/Obj, Fw/Port, Fw/Ports/CompletionStatus, Fw/Ports/Ready, Fw/Ports/Signal, Fw/Ports/SuccessCondition, Fw/Prm, Fw/SerializableFile/test/TestSerializable, Fw/Sm, Fw/Test, Fw/Types/GTest, Os/Models, Svc/Cycle, Svc/DpPorts, Svc/Fatal, Svc/FatalHandler, Svc/FileDownlinkPorts, Svc/FprimeProtocol, Svc/Interfaces, Svc/PassiveConsoleTextLogger, Svc/Ping, Svc/PolyIf, Svc/Ports/CommsPorts, Svc/Ports/FilePorts, Svc/Ports/OsTimeEpoch, Svc/Ports/TlmPacketizerPorts, Svc/Ports/VersionPorts, Svc/Sched, Svc/Seq, Svc/Subtopologies/CdhCore, Svc/Subtopologies/ComCcsds, Svc/Subtopologies/ComFprime, Svc/Subtopologies/ComLoggerTee, Svc/Subtopologies/DataProducts, Svc/Subtopologies/FileHandling, Svc/Types/TlmPacketizerTypes, Svc/WatchDog, TestDeploymentsProject/Ref/PingReceiver, TestDeploymentsProject/Ref/RecvBuffApp, TestDeploymentsProject/Ref/SendBuffApp, TestDeploymentsProject/Ref/Top, TestDeploymentsProject/Ref/TypeDemo, cmake/test/data/TestDeployment/TestBuildAutocoder, cmake/test/data/TestDeployment/TestChainedAutocoder, cmake/test/data/TestDeployment/TestHeaderAutocoder, cmake/test/data/TestDeployment/TestTargetAutocoder, cmake/test/data/test-fprime-library/TestLibrary/TestComponent, cmake/test/data/test-fprime-library2/TestLibrary2/TestComponent

[devin-ai-integration]

Devin Review found 5 new potential issues.

[devin-ai-integration]

🚩 writeIn_handler reports FW_STATUS_DONE_WRITE even when writeBufferToFile fails

At Svc/FileWorker/FileWorker.cpp:182, writeDoneOut_out always reports FW_STATUS_DONE_WRITE regardless of whether writeBufferToFile returned false (indicating a write failure). This is pre-existing behavior unchanged by this PR, but it means callers cannot distinguish a successful write from a failed one via the done port status. The only indicator of failure is the WriteFileError event emitted inside writeToFile.

(Refers to line 182)

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🟡 New InvalidInput event lacks throttle, unlike all other externally-triggered warning high events

The new InvalidInput event is declared as severity warning high but has no throttle or every clause. Every other externally-triggered warning high event in this component (NotInIdle, CrcFailed, ReadFailedFileSize, OpenFileError, WriteFileError, WriteValidationOpenError, WriteValidationReadError) uses throttle 2 every {seconds=10}. Since InvalidInput fires before any state transition (i.e. the component remains in IDLE), a caller repeatedly sending malformed async requests can flood the event log without any rate limiting. This is the exact scenario the throttle mechanism was designed to prevent, and exactly why NotInIdle (which also fires pre-state-check) includes it.

Suggested change

	@ Invalid input arguments received on a port handler
	event InvalidInput(
	handler: string size 16
	issue: string size 32
	) severity warning high \
	format "Invalid input in {} handler: {}"
	@ Invalid input arguments received on a port handler
	event InvalidInput(
	handler: string size 16
	issue: string size 32
	) severity warning high \
	format "Invalid input in {} handler: {}" \
	throttle 2 \
	every {seconds=10}

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 FPP comment style change is a significant formatting regression

The section header comments in FileWorker.fpp were changed from properly indented # ------ style to flush-left #-- -- -- style. While both are valid FPP comments, the new style breaks the visual indentation hierarchy within the active component block and uses a non-standard dash-space pattern (-- -- --) instead of the continuous dashes (------) used throughout other .fpp files in the repository. This appears to be an accidental reformatting, possibly by an automated tool.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 Pre-existing: readBufferFromFile always returns FW_STATUS_DONE_READ even on read errors

The readBufferFromFile function at line 191-216 always returns FW_STATUS_DONE_READ as long as the file was opened successfully, regardless of whether readFile encountered a read error, abort, or timeout inside readFileBytes. The readIn_handler at line 85 then passes this status to readDoneOut_out. This means callers receive FW_STATUS_DONE_READ even when the read didn't fully complete. This is a pre-existing issue not introduced by this PR, but the PR's changes (adding validation and the getFileSize error path) make the error-handling picture more complete without addressing this gap.

(Refers to lines 82-86)

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 Edge case: offsetBytes == buffer.getSize() passes validation but triggers downstream assert

In writeIn_handler, the validation at line 141 checks offsetBytes > buffer.getSize(). When offsetBytes == buffer.getSize(), it passes validation, but downstream in writeBufferToFile -> writeToFile, the computed size becomes 0, which triggers FW_ASSERT(size > 0) at Svc/FileWorker/FileWorker.cpp:466. This is a pre-existing edge case (the old FW_ASSERT(offsetBytes <= buffer.getSize()) had the same semantics), but now that the handler is doing explicit input validation, it might be worth also rejecting offsetBytes == buffer.getSize() or handling the zero-byte-write case.

---

Was this helpful? React with 👍 or 👎 to provide feedback.