Update Coding_Conventions_and_Standards.md (#7705)

docs/Coding_Conventions_and_Standards.md

@@ -109,6 +109,19 @@ void foo(gsl::span<const std::string> names) {
 * Use [SafeInt](https://github.com/dcleblanc/SafeInt) when calculating the size of memory to allocate to protect against overflow errors
   * `#include "core/common/safeint.h"`
   * search for `SafeInt<size_t>` in the code for examples
+* The following C++ warnings should never be disabled in onnxruntime VC++ projects(Required by [Binskim](https://github.com/microsoft/binskim/blob/d9afb65c89a621411efded74c27999281d87867e/src/BinSkim.Rules/PERules/BA2007.EnableCriticalCompilerWarnings.cs)).
+  1. [4018](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4018) 'token' : signed/unsigned mismatch
+  2. [4146](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4146?view=msvc-160) unary minus operator applied to unsigned type, result still unsigned
+  3. [4244](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4244?view=msvc-160) 'argument' : conversion from 'type1' to 'type2', possible loss of data. For example, casting a int64_t to size_t. 
+  4. [4267](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4267?view=msvc-160) 'var' : conversion from 'size_t' to 'type', possible loss of data.
+  5. [4302](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4302?view=msvc-160) 'conversion' : truncation from 'type 1' to 'type 2'
+  6. [4308](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4308?view=msvc-160) negative integral constant converted to unsigned type 
+  7. [4532](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4532?view=msvc-160) 'continue' : jump out of \_\_finally/finally block has undefined behavior during termination handling
+  8. [4533](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4533?view=msvc-160) initialization of 'variable' is skipped by 'instruction'
+  9. [4700](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-and-level-4-c4700?view=msvc-160) uninitialized local variable 'name' used
+  10. [4789](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4789?view=msvc-160) buffer 'identifier' of size N bytes will be overrun; M bytes will be written starting at offset L
+  11. [4995](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4995?view=msvc-160) 'function': name was marked as #pragma deprecated 
+  12. [4996](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-160) Your code uses a function, class member, variable, or typedef that's marked deprecated
 
 #### Clang-format
 
@@ -122,6 +135,8 @@ Visual Studio Code Analysis with [C++ Core guidelines](https://github.com/isocpp
 
 Code changes should build with no Code Analysis warnings, however this is somewhat difficult to achieve consistently as the Code Analysis implementation is in fairly constant flux. Different minor releases may have less false positives (a build with the latest version may be warning free, and a build with an earlier version may not), or detect additional problems (an earlier version builds warning free and a later version doesn't).
 
+We use [BinSkim Binary Analyzer](https://github.com/microsoft/binskim) to scan our binraries.
+
 ## Unit Testing and Code Coverage
 
 There should be unit tests that cover the core functionality of the product, expected edge cases, and expected errors.

docs/cmake_guideline.md

@@ -31,6 +31,69 @@ On Linux, it matters when one static library references another.
 
 So, in general, please always put them in right order (according to their dependency relationship).
 
+Example:
+
+CMakeLists.txt:
+```cmake
+project(test1 C CXX)
+
+add_library(test1 STATIC test1.c)
+add_library(test2 STATIC test2.c)
+add_executable(test3 main.cpp)
+target_link_libraries(test3 PRIVATE test1 test2)
+```
+
+test1.c:
+```c
+#include <stdio.h>
+
+void foo(){
+  printf("hello foo\n");
+}
+```
+
+test2.c:
+```c
+#include <stdio.h>
+
+extern void foo();
+
+void foo2(){
+  foo();
+  printf("hello foo2\n");
+}
+```
+
+main.cpp
+```c++
+#include <iostream>
+
+extern "C" {
+  extern void foo2();
+}
+
+int main(){
+  foo2();
+  return 0;
+}
+```
+Then when you build the project, it will report
+```
+/usr/bin/ld: libtest2.a(test2.c.o): in function `foo2':
+test2.c:(.text+0xa): undefined reference to `foo'
+collect2: error: ld returned 1 exit status
+```
+But if you change
+```cmake
+target_link_libraries(test3 PRIVATE test1 test2)
+```
+to 
+```cmake
+target_link_libraries(test3 PRIVATE test2 test1)
+```
+It will be fine.
+
+Or if you change the two libraries to shared libs, build will also pass.
 
 # Don't call target\_link\_libraries on static libraries
 You could do it, but please don't.
@@ -72,7 +135,41 @@ Use the first one, because the second one is deprecated. Don't use ["find\_packa
 
 So, be careful on this when you copy code from another project to ours, the changes may not work.
 
-
+# Basics of cross-compiling
+Host System: The system where compiling happens
+Target System: The system where built programs and libraries will run.
+
+Here system means the combination of
+
+- CPU Arch: x86_32, x86_64, armv6, armv7, arvm7l, aarch64, …
+- OS: bare-metal, linux, Windows
+- Libc: gnu libc/ulibc/musl/…
+- ABI: ARM has mutilple ABIs like eabi, eabihf…
+
+When "host system" != "target system" (any different in the four dimensions), we call it cross-compiling. For example, when you build a Windows EXE on Linux, or build an ARM program on an x86_64 CPU, you are doing cross-compiling. Then special handling is needed.
+
+For example, while you build the whole code base for the target system, you must build protoc for the host system. And because protoc also depends on libprotobuf, you will need to build libprotobuf twice: for the host and for the target.
+
+Here we focus on what you should know when authoring cmake files.
+
+# How to determine the host CPU architecture: on which cmake is running
+CMAKE_HOST_SYSTEM_PROCESSOR is the one you should use. 
+
+What are the valid values:
+- macOS: it can be x86_64 or arm64. (maybe it could also be arm64e but cmake forgot to document that)
+- Linux: i686, x86_64, aarch64, armv7l, ... The possible values for `uname -m` command. They sightly differ from what you can get from GCC. This sometimes confuses people: `cmake` and `uname` sit in one boat, GCC is in another boat but GCC is closer to your C/C++ source code.
+- Windows: AMD64, ...
+- Android/iOS/...: we don't care. We don't use them as a development environment.
+
+# How to determine what CPU architecture(or architectures) you are building for
+
+Linux: Use `CMAKE_SYSTEM_PROCESSOR`. When not cross-compiling, you can read this value but please don't set it. This variable has the same value as `CMAKE_HOST_SYSTEM_PROCESSOR`. When cross-compiling, a CMAKE_TOOLCHAIN_FILE should set the `CMAKE_SYSTEM_PROCESSOR` variable to match target architecture. But, I used to forgot it and it didn't cause any problem until pytorch cpuinfo was added into onnx runtime as a dependency. For simplicity, let's assume people won't miss it.
+
+macOS: Don't use `CMAKE_SYSTEM_PROCESSOR`. First, please check if target property `OSX_ARCHITECTURES` or `CMAKE_OSX_ARCHITECTURES` was set. Please note it is a list which could contain multiple values, like "arm64;x86_64". Otherwise please use`CMAKE_HOST_SYSTEM_PROCESSOR`. 
+
+In all cases, possible values are the same as what we talked above(for `CMAKE_HOST_SYSTEM_PROCESSOR`).
+
+If you have platform-specific code, you can wrap it with conditional compilation macros("#ifdef"). But if you have a file that is solely for one specific cpu architecture without the macros, you need to put it in a separated lib, like libfoo_x86, libfoo_arm64, ...